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

Remove outer div (fixes #19) #725

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

alexrsagen
Copy link

@alexrsagen alexrsagen commented Feb 14, 2022

What kind of change does this PR introduce?

  • Adds properties width and fieldClassName to the <SemanticDatepicker> element, forwarding them in addition to the existing properties disabled, error, inline, required to the existing <Form.Field> element.
  • Wraps the <Form.Field> element with a <Ref> element, to set the ref this.el on the <Form.Field> element
  • Sets position: relative CSS on the existing <Form.Field> element
  • Removes the outer div

What is the current behavior?
You cannot set className or width on the inner <Form.Field> element.

What is the new behavior?
You can now set className or width on the inner <Form.Field> element.

There is also one less <div> in the DOM, which is nice.

Checklist:

  • Documentation
  • Tests
  • Ready to be merged

I'm not quite sure if there are any additional steps needed before merging this, so I am requesting a review before anything is merged.

@vercel
Copy link

vercel bot commented Feb 14, 2022

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/arthurdenner/react-semantic-ui-datepickers/AVs7giEbj7ZGyQrVDNRTNfH2S5we
✅ Preview: https://react-semantic-ui-datepickers-git-fork-kons-79c053-arthurdenner.vercel.app

@alexrsagen
Copy link
Author

alexrsagen commented Feb 14, 2022

@arthurdenner This should resolve #26 in an ideal way as well, since you can set the semantic <Form.Field> property width directly, right?

Hoping to get this merged and released ASAP. Thanks for your (and all the other contributors) hard work on this datepicker 😄

@stale
Copy link

stale bot commented Apr 17, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 17, 2022
@bmartin1134
Copy link

When is this being rolled into the main branch?

@jatls
Copy link

jatls commented Feb 11, 2023

I'm also keen to see this PR merged. Any updates?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants