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

Fix/styles and types cleanup #491

Merged
merged 11 commits into from
Aug 12, 2021
Merged

Conversation

aappoalander
Copy link
Contributor

@aappoalander aappoalander commented Aug 2, 2021

Description

This maintenance PR removes redundant abstractions and typings and simplifies styles handling. Theme handling is refactored to use a provider pattern instead of constant theme. In additions, some minor improvements to folder structure are introduced.

All snapshots remain unchanged.

Motivation and Context

Current styles and types implementations date back over two years to situation where the goal was to have an unstyled abstraction of all components. This is no longer the goal and most of these abstractions have become redundant.

To simplify the codebase, the theme and tokens handling had to be refactored. Theme is now provided via context and can be customized when needed. Components only use the theme via the context and do not import anything else from the theme directly. Theme utils are still used directly, but style related common tools are now located under utils.

In addition, css-in-to-js had multiple vulnerabilities due to no updates and was removed as inspired by PR:#489

Removing of custom Reach-UI typings resulted in simplified implementation of LanguageMenu, which no longer supports custom components or menu popover class names.

How Has This Been Tested?

Tested on MacOS using Chrome, Edge and Safari with VoiceOver. Tested using styleguidist build, CRA-TS and SSR test applications via verdaccio. In addition, tested with styleguidist using Android and Chrome.

Release notes

Theme and styles:

  • Add export for suomifiTheme and related interfaces
  • Add and export SuomifiThemeProvider and related documentation
  • Remove css.in-to-js dependency and replace it with static object
  • Refactor theme generation and remove redundant focus utils. Include derived tokens to theme.
    Directory structure:
  • Remove unused login icon and directory
  • Move styleguidist components to docs directory
  • Cleanup utils directory structure
    Types:
  • Remove all custom Reach-UI types and replace with the ones exported by Reach-UI
    LanguageMenu:
  • Breaking changes: Remove languageMenuPopoverProps and languageMenuPopoverComponent from LanguageMenuProps, remove related interfaces, remove popover and popoverLang classNames from internal styles.

@aappoalander aappoalander added enhancement New feature or request DX Refactoring for maintainability dependencies Pull requests that update a dependency file labels Aug 2, 2021
@aappoalander aappoalander self-assigned this Aug 2, 2021
src/core/theme/index.ts Outdated Show resolved Hide resolved
src/core/theme/index.ts Outdated Show resolved Hide resolved
@ketsappi
Copy link
Contributor

ketsappi commented Aug 9, 2021

Remembered the old issue #225 that can be closed when this PR is ready.

Copy link
Contributor

@ketsappi ketsappi left a comment

Choose a reason for hiding this comment

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

Left some comments here and there but none shouldn't be any real problem. Suggestions here and there to open discussion.

Will next test library with test benches.

.styleguidist/themevalues.md Outdated Show resolved Hide resolved
src/core/theme/SuomifiTheme/SuomifiTheme.ts Outdated Show resolved Hide resolved
src/index.tsx Show resolved Hide resolved
src/core/Dropdown/Dropdown/Dropdown.baseStyles.tsx Outdated Show resolved Hide resolved
src/core/theme/SuomifiTheme/SuomifiTheme.ts Outdated Show resolved Hide resolved
src/core/Modal/ModalContent/ModalContent.tsx Outdated Show resolved Hide resolved
src/core/utils/AutoId/AutoId.tsx Show resolved Hide resolved
Copy link
Contributor

@ketsappi ketsappi left a comment

Choose a reason for hiding this comment

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

Seems to work nicely when smoke tested in CRA-TS and next.js projects.

Good work!

@LJKaski LJKaski self-requested a review August 12, 2021 05:38
Copy link
Collaborator

@LJKaski LJKaski left a comment

Choose a reason for hiding this comment

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

A lot of changes for sure, but seems to work as advertised. Good work :)

@aappoalander aappoalander mentioned this pull request Oct 4, 2021
@aappoalander aappoalander deleted the fix/styles-and-types-cleanup branch October 13, 2021 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file DX Refactoring for maintainability enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants