Skip to content

Commit

Permalink
fix: typescript errors, optimize icons (#1967)
Browse files Browse the repository at this point in the history
* chore(deps): lock file maintenance

* fix: explicitly map icon colors, to avoid unchecked types

* refactor: clean up icons IDs

* fix: typescript errors

* docs: changeset

* refactor: do not use id selectors in icons

* docs: changeset

* refactor(svgr): plugin prefixIds is enabled by default

Co-authored-by: Renovate Bot <[email protected]>
Co-authored-by: Nicola Molinari <[email protected]>
  • Loading branch information
3 people authored Jul 29, 2021
1 parent 52cd68b commit 64e4bc8
Show file tree
Hide file tree
Showing 186 changed files with 8,683 additions and 7,107 deletions.
19 changes: 19 additions & 0 deletions .changeset/old-mirrors-poke.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
'@commercetools-uikit/avatar': patch
'@commercetools-uikit/secondary-button': patch
'@commercetools-uikit/field-errors': patch
'@commercetools-uikit/field-label': patch
'@commercetools-uikit/icons': patch
'@commercetools-uikit/checkbox-input': patch
'@commercetools-uikit/multiline-text-input': patch
'@commercetools-uikit/number-input': patch
'@commercetools-uikit/password-input': patch
'@commercetools-uikit/radio-input': patch
'@commercetools-uikit/rich-text-utils': patch
'@commercetools-uikit/text-input': patch
'@commercetools-uikit/tag': patch
'@commercetools-uikit/tooltip': patch
'@commercetools-uikit/hooks': patch
---

Fix TypeScript export declarations from package entry point. Migrate missing TS files.
11 changes: 11 additions & 0 deletions .changeset/rude-suits-grin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
'@commercetools-uikit/data-table': patch
'@commercetools-uikit/icons': patch
'@commercetools-uikit/checkbox-input': patch
'@commercetools-uikit/radio-input': patch
'@commercetools-uikit/rich-text-utils': patch
---

Fix generated icon components: selected colors are not explicitly mapped, unused IDs are removed (via SVGO), do not use IDs as style selectors.

This is an internal refactoring and should not affect the usage of the components.
17 changes: 17 additions & 0 deletions .changeset/wild-turkeys-boil.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
'@commercetools-uikit/avatar': patch
'@commercetools-uikit/secondary-button': patch
'@commercetools-uikit/field-errors': patch
'@commercetools-uikit/field-label': patch
'@commercetools-uikit/icons': patch
'@commercetools-uikit/multiline-text-input': patch
'@commercetools-uikit/number-input': patch
'@commercetools-uikit/password-input': patch
'@commercetools-uikit/text-input': patch
'@commercetools-uikit/tag': patch
'@commercetools-uikit/tooltip': patch
'@commercetools-uikit/hooks': patch
---

Some files were not migrated to TypeScript. Also, each package entry point should not contain any TypeScript syntax (as it does not play well with preconstruct).
Instead, explicit export types are defined in a `export-types.ts` file, which is then re-exported from the entry point.
2,827 changes: 1,585 additions & 1,242 deletions docs/yarn.lock

Large diffs are not rendered by default.

14 changes: 13 additions & 1 deletion lint-staged.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,19 @@ module.exports = {
'*.md': ['prettier --write --parser markdown'],
'*.yaml': ['prettier --write --parser yaml'],
'*.json': ['prettier --write --parser json'],
'*.{js,ts,tsx}': [
'*.js': [
'yarn prettier --write',
// NOTE: apparently if you pass some argument that is not a flag AFTER the `reporters`
// flag, jest does not seem correctly parse the arguments.
//
// No tests found related to files changed since last commit.
// Run Jest without `-o` or with `--all` to run all tests.
// Error: An error occurred while adding the reporter at path "/path/to/file".Reporter is not a constructor
//
// For that reason, we move the `--onlyChanged` flag next to it.
'yarn lint:js --reporters=jest-silent-reporter --onlyChanged',
],
'*.{ts,tsx}': [
'yarn prettier --write',
// NOTE: apparently if you pass some argument that is not a flag AFTER the `reporters`
// flag, jest does not seem correctly parse the arguments.
Expand Down
3 changes: 3 additions & 0 deletions packages/components/avatar/src/export-types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import type { TAvatarProps as AvatarProps } from './avatar';

export type TAvatarProps = AvatarProps;
5 changes: 1 addition & 4 deletions packages/components/avatar/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
import type { TAvatarProps as AvatarProps } from './avatar';

export { default } from './avatar';
export { default as version } from './version';

export type TAvatarProps = AvatarProps;
export * from './export-types';
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
import type { Theme } from '@emotion/react';

import { warning } from '@commercetools-uikit/utils';
import { css } from '@emotion/react';
import { customProperties as vars } from '@commercetools-uikit/design-system';

const getStateStyles = (isDisabled, isActive, theme) => {
const getStateStyles = (
isDisabled: boolean,
isActive: boolean,
theme: Theme
) => {
if (isDisabled) {
const baseDisabledStyles = css`
box-shadow: 0 0 0 1px ${vars.colorNeutral} inset;
Expand Down Expand Up @@ -63,7 +69,7 @@ const getStateStyles = (isDisabled, isActive, theme) => {
`;
};

const getThemeStyles = (theme) => {
const getThemeStyles = (theme: Theme) => {
if (!theme) return css``;

if (theme === 'default') return css``;
Expand Down
8 changes: 6 additions & 2 deletions packages/components/data-table/src/header-cell.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,13 @@ const HeaderCell = (props) => {
<AngleUpDownIcon
size="medium"
color="surface"
id="nonActiveSortingIcon"
data-icon-state="inactive"
/>
<SortingIcon
size="medium"
color="surface"
data-icon-state="active"
/>
<SortingIcon size="medium" color="surface" id="activeSortingIcon" />
</>
)}
</HeaderCellInner>
Expand Down
30 changes: 15 additions & 15 deletions packages/components/data-table/src/header-cell.styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,38 +16,38 @@ const getButtonStyle = () => css`
font-family: inherit;
`;

/* A sortable header has the two arrow svg icons
* GIVEN column is sortable and is not focused
* THEN AngleUpDown icon is shown (default behaviour)
* AND AngleUp or AngleDown icon is not shown
*
* GIVEN column is sortable and foucsed
* THEN AngleUpDown icon is hidden
* AND AngleUp or AngleDown icon is shown
*/
const getSortableHeaderStyles = (props) => css`
width: 100%;
display: flex;
align-items: center;
/* A sortable header has the two arrow svg icons
* GIVEN column is sortable and is not focused
* THEN AngleUpDown icon is shown (default behaviour)
* AND AngleUp or AngleDown icon is not shown
*
* GIVEN column is sortable and foucsed
* THEN AngleUpDown icon is hidden
* AND AngleUp or AngleDown icon is shown
*/
svg[id='nonActiveSortingIcon'],
svg[id='activeSortingIcon'] {
svg[data-icon-state='inactive'],
svg[data-icon-state='active'] {
margin-left: ${vars.spacingS};
flex-shrink: 0;
}
svg[id='nonActiveSortingIcon'] {
svg[data-icon-state='inactive'] {
display: ${props.isActive ? 'none' : 'inline-block'};
}
svg[id='activeSortingIcon'] {
svg[data-icon-state='active'] {
display: ${props.isActive ? 'inline-block' : 'none'};
}
:hover,
:focus {
svg[id='nonActiveSortingIcon'] {
svg[data-icon-state='inactive'] {
display: none;
}
svg[id='activeSortingIcon'] {
svg[data-icon-state='active'] {
display: inline-block;
* {
fill: ${vars.colorNeutral};
Expand Down
7 changes: 7 additions & 0 deletions packages/components/field-errors/src/export-types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import type {
TFieldErrorsProps as FieldErrorsProps,
TFieldErrors as FieldErrors,
} from './field-errors';

export type TFieldErrorsProps = FieldErrorsProps;
export type TFieldErrors = FieldErrors;
2 changes: 1 addition & 1 deletion packages/components/field-errors/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
export { default } from './field-errors';
export { default as version } from './version';
export type { TFieldErrorsProps, TFieldErrors } from './field-errors';
export * from './export-types';
3 changes: 3 additions & 0 deletions packages/components/field-label/src/export-types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import type { TFieldLabelProps as FieldLabelProps } from './field-label';

export type TFieldLabelProps = FieldLabelProps;
4 changes: 1 addition & 3 deletions packages/components/field-label/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import type { TFieldLabelProps as FieldLabelProps } from './field-label';

export { default } from './field-label';
export { default as version } from './version';
export type TFieldLabelProps = FieldLabelProps;
export * from './export-types';
2 changes: 1 addition & 1 deletion packages/components/icons/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"main": "dist/commercetools-uikit-icons.cjs.js",
"module": "dist/commercetools-uikit-icons.esm.js",
"preconstruct": {
"entrypoints": ["./index.ts", "./inline-svg.tsx"]
"entrypoints": ["./index.ts", "./inline-svg/index.ts"]
},
"files": ["dist", "inline-svg"],
"scripts": {
Expand Down
65 changes: 41 additions & 24 deletions packages/components/icons/src/generated/AngleDownReact.tsx

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

66 changes: 41 additions & 25 deletions packages/components/icons/src/generated/AngleLeftReact.tsx

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

1 comment on commit 64e4bc8

@vercel
Copy link

@vercel vercel bot commented on 64e4bc8 Jul 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.