Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

COM-85: Refactor Theming of Picker Components #1471

Conversation

jamesricky
Copy link
Contributor

No description provided.

@jamesricky jamesricky self-assigned this Nov 30, 2023
Before importing `@emotion/styled` the following typescript error would
occur when defining some styled-components, e.g. `Root` in `packages/admin/admin-date-time/src/dateRangePicker/DateRangePicker.slots.ts`:

```error
TS2742: The inferred type of 'Root' cannot be named without a reference to '.pnpm/@emotion+styled@11.10.5_@babel+core@7.20.12_@emotion+react@11.9.3_@types+react@17.0.53_react@17.0.2/node_modules/@emotion/styled'. This is likely not portable. A type annotation is necessary.
```
@jamesricky jamesricky marked this pull request as ready for review December 7, 2023 11:10
Copy link
Collaborator

@johnnyomair johnnyomair left a comment

Choose a reason for hiding this comment

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

It would be better to have smaller PRs for separate components.

@jamesricky
Copy link
Contributor Author

It would be better to have smaller PRs for separate components.

Sorry, yes I agree. In this case it was a bit difficult, because they all used InputWithPopper as a base.
Refactoring the base required refactoring the others for everything to work without lint-errors.

@jamesricky jamesricky requested a review from johnnyomair January 3, 2024 06:27
@jamesricky jamesricky requested a review from thomasdax98 January 3, 2024 08:18
@jamesricky jamesricky merged commit b87c3c2 into feature/refactor-admin-component-theming Jan 4, 2024
@jamesricky jamesricky deleted the refactor-theming-of-picker-components branch January 4, 2024 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants