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(Switch, RadioButton, Checkbox, ProgressBar, ProgressBarStepped, Slider): create inverse variant #1277

Merged
merged 7 commits into from
Oct 31, 2024
Merged

feat(Switch, RadioButton, Checkbox, ProgressBar, ProgressBarStepped, Slider): create inverse variant #1277

merged 7 commits into from
Oct 31, 2024

Conversation

marcoskolodny
Copy link
Collaborator

Issue: Link

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 22, 2024 •
edited
Loading

Size stats

master this branch diff
Total JS 12.2 MB 12.2 MB +4.55 kB
JS without icons 1.95 MB 1.96 MB +4.55 kB
Lib overhead 68.3 kB 68.7 kB +381 B
Lib overhead (gzip) 16.6 kB 16.7 kB +59 B

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 22, 2024 •
edited
Loading

Accessibility report
✔️ No issues found

ℹ️ You can run this locally by executing yarn audit-accessibility.

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 22, 2024 •
edited
Loading

Deploy preview for mistica-web ready!

✅ Preview
https://mistica-bw3nltd7q-flows-projects-65bb050e.vercel.app

Built with commit 3b56f69.
This pull request is being automatically deployed with vercel-action

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 22, 2024 •
edited
Loading

Screenshot tests report

✔️ All passing

@marcoskolodny marcoskolodny changed the title feat(Switch, RadioButton, Checkbox, ProgressBar): create inverse variant feat(Switch, RadioButton, Checkbox, ProgressBar, ProgressBarStepped): create inverse variant Oct 22, 2024
marcoskolodny
marcoskolodny commented Oct 22, 2024
View reviewed changes
..._screenshot_tests__/__image_snapshots__/slider-screenshot-test-tsx-slider-desktop-1-snap.png Outdated
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Slider screenshots changed because I've updated the spacing in the story (it was inconsistent in the controlled and uncontrolled stories).

@marcoskolodny marcoskolodny marked this pull request as ready for review October 22, 2024 16:04
yceballost
yceballost reviewed Oct 22, 2024
View reviewed changes
src/skins/o2-new.tsx Outdated
text5: {weight: 'bold', size: {mobile: 20, desktop: 28}, lineHeight: {mobile: 24, desktop: 32}},
text6: {weight: 'bold', size: {mobile: 24, desktop: 32}, lineHeight: {mobile: 32, desktop: 40}},
text7: {weight: 'bold', size: {mobile: 28, desktop: 40}, lineHeight: {mobile: 32, desktop: 48}},
text8: {weight: 'bold', size: {mobile: 32, desktop: 48}, lineHeight: {mobile: 40, desktop: 56}},
text9: {weight: 'bold', size: {mobile: 40, desktop: 56}, lineHeight: {mobile: 48, desktop: 64}},
text10: {weight: 'bold', size: {mobile: 48, desktop: 64}, lineHeight: {mobile: 56, desktop: 72}},
text1: {size: {mobile: 12, desktop: 14}, lineHeight: {mobile: 16, desktop: 20}},
Copy link
Contributor

Choose a reason for hiding this comment

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

:S

why this change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's just a change in the order of lines. Every time I've updated this file, I was fixing this order by hand so it looks "nicer", but I think it's better to just leave it as it is, so we forget about this useless diff.

Copy link
Collaborator

Choose a reason for hiding this comment

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

why don't change the generation script to fix the order?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've created a task to review this: https://jira.tid.es/browse/WEB-2093

For now, I've fixed them manually

@yceballost
Copy link
Contributor

include Slider in PR title 😛

@marcoskolodny marcoskolodny changed the title feat(Switch, RadioButton, Checkbox, ProgressBar, ProgressBarStepped): create inverse variant feat(Switch, RadioButton, Checkbox, ProgressBar, ProgressBarStepped, Slider): create inverse variant Oct 23, 2024
atabel
atabel approved these changes Oct 25, 2024
View reviewed changes
...hots__/private-skin-components-screenshot-test-tsx-components-in-movistar-inverse-3-snap.png Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

the stepper is still broken over inverse, do we have plans for this in the future @yceballost ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've mentioned this to design team and they said they will iterate on this in the future

...shots__/private-skin-components-screenshot-test-tsx-components-in-o-2-new-inverse-2-snap.png Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmmm, why did the whole background color changed here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My guess is because of small rendering differences when generating the new image (the background is a gradient, if there is any small difference, it will show as if the entire background changed because all the pixels will be moved by a very small amount).

src/skins/o2-new.tsx Outdated
text5: {weight: 'bold', size: {mobile: 20, desktop: 28}, lineHeight: {mobile: 24, desktop: 32}},
text6: {weight: 'bold', size: {mobile: 24, desktop: 32}, lineHeight: {mobile: 32, desktop: 40}},
text7: {weight: 'bold', size: {mobile: 28, desktop: 40}, lineHeight: {mobile: 32, desktop: 48}},
text8: {weight: 'bold', size: {mobile: 32, desktop: 48}, lineHeight: {mobile: 40, desktop: 56}},
text9: {weight: 'bold', size: {mobile: 40, desktop: 56}, lineHeight: {mobile: 48, desktop: 64}},
text10: {weight: 'bold', size: {mobile: 48, desktop: 64}, lineHeight: {mobile: 56, desktop: 72}},
text1: {size: {mobile: 12, desktop: 14}, lineHeight: {mobile: 16, desktop: 20}},
Copy link
Collaborator

Choose a reason for hiding this comment

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

why don't change the generation script to fix the order?

yceballost
yceballost approved these changes Oct 28, 2024
View reviewed changes
Copy link
Contributor

@yceballost yceballost left a comment

Choose a reason for hiding this comment

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

👌

@marcoskolodny marcoskolodny added this pull request to the merge queue Oct 31, 2024
Merged via the queue into master with commit 3129fb9 Oct 31, 2024
11 checks passed
@marcoskolodny marcoskolodny deleted the WEB-2084-controls-inverse-variant branch October 31, 2024 19:19
tuentisre pushed a commit that referenced this pull request Nov 4, 2024
@semantic-release-bot
chore(release): 16.4.0 [skip ci]
8d91924 
# [16.4.0](v16.3.1...v16.4.0) (2024-11-04)

### Bug Fixes

* **TextField:** adjust position of maxLength's screen reader label ([#1283](#1283)) ([fda424e](fda424e))
* **TextField:** avoid right helper text from wrapping, fix spacing and aria label for maxCount text when multiline is true ([#1272](#1272)) ([85fcb31](85fcb31))
* **useDisableBodyScroll:** avoid affecting body's height ([#1279](#1279)) ([b68f317](b68f317))

### Features

* **PhoneNumberFieldLite:** Phone number field with simple formatting to reduce bundle size ([#1276](#1276)) ([a141b97](a141b97))
* **Row:** allow aria-label in informative rows ([#1269](#1269)) ([65b5d42](65b5d42))
* **Rows, Cards, FeedbackScreen, FormFields, Buttons:** add test ids for components and their internal elements ([#1270](#1270)) ([fc63201](fc63201))
* **Spinner:** improve a11y ([#1274](#1274)) ([5267ad5](5267ad5))
* **Switch, RadioButton, Checkbox, ProgressBar, ProgressBarStepped, Slider:** create inverse variant ([#1277](#1277)) ([3129fb9](3129fb9))
@tuentisre
Copy link
Collaborator

🎉 This PR is included in version 16.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pladaria pladaria pladaria approved these changes

@atabel atabel atabel approved these changes

@yceballost yceballost yceballost approved these changes

@aweell aweell aweell approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@marcoskolodny @yceballost @tuentisre @pladaria @atabel @aweell