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(component): create Toggle component #1274

Merged
merged 1 commit into from
Aug 2, 2023
Merged

feat(component): create Toggle component #1274

merged 1 commit into from
Aug 2, 2023

Conversation

bc-apostoliuk
Copy link
Contributor

@bc-apostoliuk bc-apostoliuk commented Jul 27, 2023 •
edited
Loading

What? Why?

Create a Toggle component.

Screenshots/Screen Recordings

Screenshot 2023-08-01 at 13 52 51 Screenshot 2023-08-01 at 13 53 03 Screenshot 2023-08-01 at 13 53 49 Screenshot 2023-08-01 at 13 54 35 Screenshot 2023-08-01 at 13 56 21 Screenshot 2023-08-01 at 13 56 41 Screenshot 2023-08-01 at 13 56 51

FOCUSED:
Screenshot 2023-08-01 at 13 57 08
Screenshot 2023-08-01 at 13 57 20
Screenshot 2023-08-01 at 13 57 38
Screenshot 2023-08-01 at 13 57 48

DISABLED:
Screenshot 2023-08-01 at 13 58 50

HOVERED:
Screenshot 2023-08-01 at 13 15 15

Screenshot 2023-08-01 at 13 58 16

Testing/Proof

Locally + UT.

@bc-apostoliuk bc-apostoliuk requested review from a team as code owners July 27, 2023 14:08
jorgemoya
jorgemoya reviewed Jul 28, 2023
View reviewed changes
Copy link
Contributor

@jorgemoya jorgemoya left a comment

Choose a reason for hiding this comment

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

Great to see a new component! Just a few typos and comments.

packages/docs/PropTables/TogglePropsTable.tsx Outdated Show resolved Hide resolved
packages/docs/PropTables/TogglePropsTable.tsx Outdated Show resolved Hide resolved
packages/docs/PropTables/TogglePropsTable.tsx Outdated Show resolved Hide resolved
packages/docs/PropTables/TogglePropsTable.tsx Outdated Show resolved Hide resolved
packages/docs/pages/toggle.tsx Outdated Show resolved Hide resolved
packages/big-design/src/components/Toggle/Toggle.tsx Outdated Show resolved Hide resolved
packages/big-design/src/components/Toggle/Toggle.tsx Show resolved Hide resolved
packages/big-design/src/components/Toggle/Toggle.tsx Outdated Show resolved Hide resolved
@bc-apostoliuk bc-apostoliuk force-pushed the toggle-component branch 2 times, most recently from 417595b to 494e8af Compare July 31, 2023 10:11
@bc-apostoliuk
Copy link
Contributor Author

@jorgemoya Thanks for the comments. I have made the changes accordingly.

@bc-apostoliuk bc-apostoliuk requested a review from jorgemoya July 31, 2023 10:11
@bc-apostoliuk bc-apostoliuk force-pushed the toggle-component branch 2 times, most recently from 1ceac59 to 9803a8e Compare July 31, 2023 15:07
golcinho
golcinho reviewed Jul 31, 2023
View reviewed changes
Copy link
Contributor

@golcinho golcinho left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Great job 👍 We will remove a lot of snowflakes, thank you!

packages/big-design/src/components/Toggle/styled.ts Outdated Show resolved Hide resolved
@bc-apostoliuk bc-apostoliuk force-pushed the toggle-component branch 3 times, most recently from 6977882 to 3942187 Compare July 31, 2023 15:53
jorgemoya
jorgemoya approved these changes Jul 31, 2023
View reviewed changes
Copy link
Contributor

@jorgemoya jorgemoya left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, just need to fix tests. 👍

@bc-apostoliuk bc-apostoliuk force-pushed the toggle-component branch 2 times, most recently from 8bb2e56 to f078739 Compare August 1, 2023 08:24
vdm-kr
vdm-kr suggested changes Aug 1, 2023
View reviewed changes
Copy link

@vdm-kr vdm-kr left a comment

Choose a reason for hiding this comment

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

When the Icon type is selected, there should be other paddings, please check the mockups

@bc-apostoliuk bc-apostoliuk force-pushed the toggle-component branch 2 times, most recently from 574ed1a to c2be7bd Compare August 1, 2023 10:13
@bc-apostoliuk
Copy link
Contributor Author

@vdm-kr Thanks for noticing. I have fixed this case and updated the screenshots attached above.
✅

@bc-apostoliuk bc-apostoliuk requested a review from vdm-kr August 1, 2023 10:22
vdm-kr
vdm-kr approved these changes Aug 1, 2023
View reviewed changes
Copy link

@vdm-kr vdm-kr left a comment

Choose a reason for hiding this comment

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

LGTM

@bc-apostoliuk bc-apostoliuk merged commit 8200e44 into bigcommerce:master Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vdm-kr vdm-kr vdm-kr approved these changes

@golcinho golcinho golcinho approved these changes

@jorgemoya jorgemoya jorgemoya approved these changes

@bc-traciporter bc-traciporter Awaiting requested review from bc-traciporter bc-traciporter was automatically assigned from bigcommerce/dev-docs-team

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bc-apostoliuk @jorgemoya @golcinho @vdm-kr