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(components): Add labelPosition prop on Switch #2137

Closed
wants to merge 0 commits into from
Closed

feat(components): Add labelPosition prop on Switch #2137

wants to merge 0 commits into from

Conversation

PeterlitsZo
Copy link
Contributor

Switch has a prop label to set its label. And its position is
defaultly at the end. To put the label at the start of Switch is
hard.

So I add labelPosition, using gap: 2 instead of the mr: 2 to make
sure that those children's space is right.

By the way, I move those inner components outside, to make those code
more clear than before. I hope that works.

@vercel
Copy link

vercel bot commented Feb 18, 2022 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/systemui/theme-ui/EyeMg3oSXGWpUZ1Fa9ke7uSGdzMK
✅ Preview: https://theme-ui-git-fork-peterlitszo-add-labelposition-29a676-systemui.vercel.app

@PeterlitsZo
Copy link
Contributor Author

PeterlitsZo commented Feb 18, 2022 •
edited
Loading

For issue #2124

Weekend, right?

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 18, 2022 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit d70d526:

Sandbox Source
next-theme-ui-example Configuration
gatsby-plugin-theme-ui-example Configuration

@PeterlitsZo PeterlitsZo mentioned this pull request Feb 18, 2022
Closed
@lachlanjc lachlanjc changed the title feat(Switch): Add labelPosition for Switch. feat(components): Add labelPosition prop on Switch Feb 18, 2022
@PeterlitsZo
Copy link
Contributor Author

PeterlitsZo commented Feb 18, 2022 •
edited
Loading

Well, I find that you like to use package name rather than the component name...

I will fix it with force push. (I am not sure that it is a good idea...)

I will go to sleep. See you tomorrow.

@hasparus hasparus assigned hasparus and unassigned hasparus Feb 20, 2022
@hasparus hasparus added the enhancement New feature or request label Feb 20, 2022
hasparus
hasparus requested changes Feb 20, 2022
View reviewed changes
Copy link
Member

@hasparus hasparus left a comment

Choose a reason for hiding this comment

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

https://github.com/system-ui/theme-ui/pull/2137/files#r810646380

@PeterlitsZo
Copy link
Contributor Author

OK! I will work on it tonight!

@PeterlitsZo
Copy link
Contributor Author

  • tonight X
  • after 11 days V

By the way, I find that if running yarn format, it changes a lot of files.

I think we need to commit with those files which is formatted.

lachlanjc
lachlanjc approved these changes Apr 18, 2022
View reviewed changes
Copy link
Member

@lachlanjc lachlanjc left a comment

Choose a reason for hiding this comment

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

Thanks for contributing this, sorry for the long wait! Suggested some docs improvements but the code functionality looks good.

packages/docs/src/pages/components/switch.mdx Outdated Show resolved Hide resolved
packages/docs/src/pages/components/switch.mdx Outdated Show resolved Hide resolved
packages/docs/src/pages/components/switch.mdx Outdated Show resolved Hide resolved
packages/docs/src/pages/components/switch.mdx Outdated Show resolved Hide resolved
@vercel
Copy link

vercel bot commented Apr 18, 2022 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
theme-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 10, 2023 3:27pm

@PeterlitsZo
Copy link
Contributor Author

If it is OK, please tell me to rebase the git history to get more clear history.

@chrislombaard
Copy link

Hi @PeterlitsZo @hasparus @lachlanjc , is there an update on this PR?

We're currently experiencing this issue in our project with the Switch/ Label stye props being only targetable for the Switch component and not the Label. I.e we want to either disable, or move the label to the left side of the switch. Is there another work-around for this in the meantime?

@PeterlitsZo
Copy link
Contributor Author

@chrislombaard Unbelievable that this PR is still not merged. I will deal with it in this weekend (maybe this weekend :) ).

@PeterlitsZo
Copy link
Contributor Author

Wait one more week...

@ajjdore
Copy link

ajjdore commented Aug 7, 2023

Wait one more week...
 🙏

@PeterlitsZo PeterlitsZo mentioned this pull request Aug 10, 2023
Open
@PeterlitsZo
Copy link
Contributor Author

-> Move to new PR: #2452

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

@lachlanjc lachlanjc lachlanjc approved these changes

@hasparus hasparus Awaiting requested review from hasparus

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@PeterlitsZo @chrislombaard @ajjdore @lachlanjc @hasparus