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

chore(PPDSC-1852): replace enums #340

Merged
merged 62 commits into from
Aug 26, 2022
Merged

Conversation

baburay23
Copy link
Contributor

@baburay23 baburay23 commented Aug 18, 2022

PPDSC-1852

What

  1. Background - why this is needed
  2. What did you do
    replaced enums with union types for
ButtonSize
FlagSize
MenuItemAlign
MenuItemSize
LabelPosition
Flow
StackDistribution
AlignSelfValues
TabAlign
TabSize
TabsDistribution
TabsIndicatorPosition
TagSize
TextFieldSize
AudioEvents: there is a mapping used not sure how to achieve in union types
OverrideProp: same as above
EventTrigger: it's not easy to write a codemod script for this as EventTrigger is used differently.
  1. What does the reviewers should expect

Just wanted to add some snapshots have been updated this is because I had to change the mapping from enums to an array so only the titles have changed. For menu.test.tsx it looks like there are changes however there are no visual changes because of the way tests are written the snapshots are out of place.
For each test i have written I have compared the it in https://text-compare.com/
so the previous test called Menu renders with menu items aligned at the end 1 is the exact same as the new test
Menu with align renders with menu items aligned at the end vertical 1
same with
before
Menu renders with menu items aligned at the start 2 and after Menu with align renders with menu items aligned at the start horizontal 1 are the same visually. Sorry just wanted to add this in as this threw me off originally.

I have done:

  • Written unit tests against changes
  • Written functional tests against the component and/or NewsKit site
  • Updated relevant documentation

I have tested manually:

  • The feature's functionality is working as expected on Chrome, Firefox, Safari and Edge
  • The screen reader reads and flows through the elements as expected.
  • There are no new errors in the browser console coming from this PR.
  • When visual test is not added, it renders correctly on different browsers and mobile viewports (Safari, Firefox, small mobile viewport, tablet)
  • The Playground feature is working as expected

Before:

After:

Who should review this PR:

How to test:

@newskit-engineering
Copy link
Collaborator

Copy link
Contributor

@mutebg mutebg left a comment

Choose a reason for hiding this comment

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

That was huge, I did not realise how many files we need to change to replace these enums. Good job

site/pages/components/flag.mdx Outdated Show resolved Hide resolved
site/pages/components/stack.mdx Outdated Show resolved Hide resolved
site/pages/components/stack.mdx Show resolved Hide resolved
site/pages/components/text-input.mdx Outdated Show resolved Hide resolved
src/audio-player-composable/types.ts Outdated Show resolved Hide resolved
src/form/__tests__/form-input.stories.tsx Show resolved Hide resolved
src/stack-child/types.ts Show resolved Hide resolved
src/stack/__tests__/stack.stories.tsx Outdated Show resolved Hide resolved
src/stack/__tests__/stack.test.tsx Outdated Show resolved Hide resolved
src/stack/__tests__/stack.test.tsx Outdated Show resolved Hide resolved
@mutebg mutebg changed the title Chore/ppdsc 1852 replace enums chore(PPDSC-1852): replace enums Aug 25, 2022
@baburay23 baburay23 requested a review from mutebg August 25, 2022 10:26
mutebg
mutebg previously approved these changes Aug 25, 2022
Copy link
Contributor

@Xin00163 Xin00163 left a comment

Choose a reason for hiding this comment

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

This is definitely more than 3 points and you did a great job 👍 . One tiny comment.

| 'horizontal-center'
| 'horizontal-bottom'
| 'horizontal-stretch'>"
default="vertical"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
default="vertical"
default="vertical-left"

@baburay23 baburay23 merged commit 7009f4e into main Aug 26, 2022
@baburay23 baburay23 deleted the chore/PPDSC-1852-replace-enums branch August 26, 2022 09:29
Xin00163 added a commit that referenced this pull request Oct 17, 2022
* chore(PPDSC-1852): chore-ppdse-1852-replace buttonSize with union

* chore(PPDSC-1852): chore-ppdse-1852-replace buttonSize with union

* chore(PPDSC-1852): chore-ppdse-1852-replace buttonSize with union

* chore(PPDSC-1852): chore-ppdse-1852-replace-buttonSize-with-union

* chore(PPDSC-1852): chore-ppdse-1852-replace-buttonSize-with-union

* chore(PPDSC-1852): chore-ppdse-1852-replace-menuItemAlign-with-union

* chore(PPDSC-1852): chore-ppdse-1852-replace-buttonSize-with-union

* chore(PPDSC-1852): chore-ppdse-1852-fixed audioplayer type

* chore(PPDSC-1852): chore-ppdse-1852-removed stackChild enum

* chore(PPDSC-1852): chore-ppdse-1852-removed stackChild enum

* chore(PPDSC-1852): removed Tagsize labelPosition enum

* chore(PPDSC-1852): chore-ppdse-1852-updated slider labelposition

* chore(PPDSC-1852): chore-ppdse-1852-updated src files for flow

* chore(PPDSC-1852): chore-ppdse-1852-updated slider test

* chore(PPDSC-1852): removed Tagsize tabIndicatorPosition enum

* chore(PPDSC-1852): removed MenuItemSize enum

* chore(PPDSC-1852): removed TabAlign enum

* chore(PPDSC-1852): removed tabSize enum

* chore(PPDSC-1852): chore-ppdse-1852-removed flow and stackD

* chore(PPDSC-1852): chore-ppdse-1852-stack test passing

* chore(PPDSC-1852): chore-ppdse-1852-tab align removed

* chore(PPDSC-1852): chore-ppdse-1852-got rid of tab enums

* chore(PPDSC-1852): chore-ppdse-1852-got rid of tab enums

* chore(PPDSC-1852): chore-ppdse-1852-flagsize done

* chore(PPDSC-1852): chore-ppdse-1852-buttonsize removed

* chore(PPDSC-1852): chore-ppdse-1852-updated documentation

* chore(PPDSC-1852): chore-ppdse-1852-updated tab documentation

* chore(PPDSC-1852): chore-ppdse-1852-tests updated

* chore(PPDSC-1852): chore-ppdse-1852-fixed tag size

* chore(PPDSC-1852): chore-ppdse-1852-menu align moved

* chore(PPDSC-1852): chore-ppdse-1852-tidied menu tests

* chore(PPDSC-1852): chore-ppdse-1852-fixed stack test types

* chore(PPDSC-1852): removed TabsDistribution enum

* chore(PPDSC-1852): chore-ppdse-1852-fixed menu test

* chore(PPDSC-1852): chore-ppdse-1852-fixed slider.mdx

* chore(PPDSC-1852): chore-ppdse-1852-textfiedl done

* chore(PPDSC-1852): chore-ppdse-1852-textinput done

* chore(PPDSC-1852): chore-ppdse-1852-textinput done

* chore(PPDSC-1852): chore-ppdse-1852-tidy up

* chore(PPDSC-1852): chore-ppdse-1852-fixed mdx error

* chore(PPDSC-1852): chore-ppdse-1852-fixed mdx error again

* chore(PPDSC-1852): chore-ppdse-1852-added codemod back

* chore(PPDSC-1852): chore-ppdse-1852-added codemod back and linted

* chore(PPDSC-1852): chore-ppdse-1852-fixed merge

* chore(PPDSC-1852): chore-ppdse-1852-fixed stack story

* chore(PPDSC-1852): chore-ppdse-1852-tidied audioplayer types

* chore(PPDSC-1852): chore-ppdse-1852-changed to lower case

* chore(PPDSC-1852): chore-ppdse-1852-fixed comments

* fix(PPDSC-1852): add lowercase with space to enum codemod

* chore(PPDSC-1852): chore-ppdse-1852-fixed stack types

* chore(PPDSC-1852): chore-ppdse-1852-updated tests

* chore(PPDSC-1852): chore-ppdse-1852-updated stack.mdx

Co-authored-by: Ravindren <jravindren@NTS251819MC.local>
Co-authored-by: Xin00163 <wangxin00163@yahoo.co.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review Please assist in getting this reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants