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.

Sign up for GitHub

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

Jump to bottom

feat(PPDSC-2151): Add Outline Style Presets #214

Merged
merged 56 commits into from
Jun 28, 2022
Merged

feat(PPDSC-2151): Add Outline Style Presets #214

merged 56 commits into from
Jun 28, 2022

Conversation

tbradbury
Copy link
Contributor

@tbradbury tbradbury commented May 27, 2022 •
edited
Loading

PPDSC-2151

What

  1. Background - why this is needed
  2. What did you do
  3. What does the reviewers should expect

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:

@tbradbury tbradbury added the WIP Work in progress label May 27, 2022
@github-actions github-actions bot added the feature This change contains a new feature label May 27, 2022
@newskit-engineering
Copy link
Collaborator

You can preview these changes on:
Documentation Site: http://ncu-newskit-docs.s3-website-eu-west-1.amazonaws.com/ppdsc-2151-outline
Storybook: http://ncu-newskit-docs.s3-website-eu-west-1.amazonaws.com/ppdsc-2151-outline/storybook

@tbradbury tbradbury requested a review from Xin00163 June 13, 2022 13:41
@tbradbury tbradbury added ready for review Please assist in getting this reviewed and removed Requires design review labels Jun 13, 2022
Xin00163
Xin00163 reviewed Jun 14, 2022
View reviewed changes
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.

Massive effort @tbradbury. Well done. I left one comment so far.

src/utils/__tests__/style-preset.test.tsx Show resolved Hide resolved
tbradbury added 3 commits June 20, 2022 09:19
@tbradbury
feat(PPDSC-2151): merge branch 'main' into feat/PPDSC-2151-outline
c3b3483 
# Conflicts:
#	site/components/sidebar/__tests__/__snapshots__/sidebar.test.tsx.snap
#	site/components/table/column-renderer.tsx
#	site/routes.ts
@tbradbury
feat(PPDSC-2151): update snapshots
dd98d11
@tbradbury
feat(PPDSC-2151): update pseudo state tests
cab75d9
mutebg
mutebg reviewed Jun 21, 2022
View reviewed changes
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.

Quite big effort, well done 👏
few small comments.

Do you think this is breaking change? we kind of change the default styling for a lot of components, from functional point of view is not, but from visual perspective to me seems breaking one

.storybook/preview.js Outdated Show resolved Hide resolved
site/components/outline-card/outline-card.tsx Outdated Show resolved Hide resolved
src/theme/types.ts Outdated Show resolved Hide resolved
src/tag/style-presets.ts Outdated Show resolved Hide resolved
@tbradbury
Copy link
Contributor Author

Quite big effort, well done 👏 few small comments.

Do you think this is breaking change? we kind of change the default styling for a lot of components, from functional point of view is not, but from visual perspective to me seems breaking one

I'm not sure I'd say its not as functionally its fine.

mutebg
mutebg previously approved these changes Jun 23, 2022
View reviewed changes
@tbradbury
feat(PPDSC-2151): merge branch 'main' into feat/PPDSC-2151-outline
3135d85 
# Conflicts:
#	site/components/meta/__tests__/__snapshots__/meta.test.tsx.snap
Xin00163
Xin00163 reviewed Jun 27, 2022
View reviewed changes
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.

Added two comments.
Could you please add focus state to Accordion as we agreed that we will need to add in this one because some colors are not available?

src/base-switch/base-switch.tsx Outdated Show resolved Hide resolved
src/button/__tests__/button.stories.tsx Outdated Show resolved Hide resolved
@tbradbury tbradbury merged commit 42dd2f5 into main Jun 28, 2022
@tbradbury tbradbury deleted the feat/PPDSC-2151-outline branch June 28, 2022 13:22
Xin00163 pushed a commit that referenced this pull request Oct 17, 2022
@tbradbury
feat(PPDSC-2151): Add Outline Style Presets (#214)
7a82c78 
* feat(PPDSC-2151): add outline to style Preset

* feat(PPDSC-2151): update snapshots

* feat(PPDSC-2151): interactiveFocus darktheme value

* feat(PPDSC-2151): interactiveFocus darktheme value

* feat(PPDSC-2151): update button style presets

* feat(PPDSC-2151): update icon-button style presets

* feat(PPDSC-2151): checkbox and radiobutton updated

* feat(PPDSC-2151): update snapshots

* feat(PPDSC-2151): update inputField preset

* feat(PPDSC-2151): update slider preset

* feat(PPDSC-2151): update switch preset

* feat(PPDSC-2151): update link preset

* feat(PPDSC-2151): remove safari css for link

* feat(PPDSC-2151): sort conflicts from merge

* feat(PPDSC-2151): update tab preset

* feat(PPDSC-2151): update menu preset

* feat(PPDSC-2151): update tag preset

* feat(PPDSC-2151): update switch component

* feat(PPDSC-2151): add Sarari style for consistency

* feat(PPDSC-2151): add padding to decorator

* feat(PPDSC-2151): update snapshots

* feat(PPDSC-2151): checkbox and radiobutton update

* feat(PPDSC-2151): update inputField focus

* feat(PPDSC-2151): update safari style for darkmode

* feat(PPDSC-2152): add documentation for outline

* feat(PPDSC-2151): fix axe error

* feat(PPDSC-2151): update style-presets page

* feat(PPDSC-2151): add storybook outline examples

* feat(PPDSC-2151): add new images

* feat(PPDSC-2151): remove coma

* feat(PPDSC-2151): update snapshots

* feat(PPDSC-2151): fix typos

* feat(PPDSC-2151): update snapshots

* feat(PPDSC-2151): update pseudo state tests

* feat(PPDSC-2151): update snapshots

* feat(PPDSC-2151): update SVG components

* feat(PPDSC-2151): remove padding from storybook

* feat(PPDSC-2151): refactor call

* feat(PPDSC-2151): change from outline to outlines

* feat(PPDSC-2151): defaultFocusVisible added

* feat(PPDSC-2151): update link

* feat(PPDSC-2151): add outline to accordion

* feat(PPDSC-2151): changes based on review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Xin00163 Xin00163 Xin00163 approved these changes

@mutebg mutebg mutebg approved these changes

@jps jps Awaiting requested review from jps

@pdimova pdimova Awaiting requested review from pdimova

@baburay23 baburay23 Awaiting requested review from baburay23

@Vanals Vanals Awaiting requested review from Vanals

@evgenitsn evgenitsn Awaiting requested review from evgenitsn

@Srinivasan-Ramasamy Srinivasan-Ramasamy Awaiting requested review from Srinivasan-Ramasamy

Assignees
No one assigned
Labels
feature This change contains a new feature ready for review Please assist in getting this reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tbradbury @newskit-engineering @mutebg @Xin00163