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

Create InputGroup component (#430) #443

Merged
merged 1 commit into from
May 3, 2023
Merged

Create InputGroup component (#430) #443

merged 1 commit into from
May 3, 2023

Conversation

dacerondrej
Copy link
Contributor

@dacerondrej dacerondrej commented Jan 10, 2023 •
edited
Loading

Any suggestions, especially regarding CSS and documentation, are warmly welcome. @adamkudrna

Build failed due to asset size limit. I will not change the limit for now as it is solved in simultaneous pull requests which will be probably merged before this one.

Closes #430

mbohal
mbohal requested changes Jan 11, 2023
View reviewed changes
Copy link
Contributor

@mbohal mbohal left a comment

Choose a reason for hiding this comment

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

Nicely done!

src/lib/components/InputGroup/__tests__/InputGroup.test.jsx Outdated Show resolved Hide resolved
src/lib/components/InputGroup/__tests__/InputGroup.test.jsx Show resolved Hide resolved
src/lib/components/InputGroup/__tests__/InputGroup.test.jsx Outdated Show resolved Hide resolved
src/lib/components/InputGroup/__tests__/InputGroup.test.jsx Show resolved Hide resolved
src/lib/components/InputGroup/README.mdx Show resolved Hide resolved
src/lib/components/Button/Button.jsx Show resolved Hide resolved
adamkudrna
adamkudrna reviewed Feb 20, 2023
View reviewed changes
src/lib/components/InputGroup/InputGroup.jsx Outdated Show resolved Hide resolved
src/lib/components/SelectField/SelectField.scss Outdated Show resolved Hide resolved
src/lib/components/TextField/TextField.scss Outdated Show resolved Hide resolved
src/lib/components/InputGroup/InputGroup.jsx Outdated Show resolved Hide resolved
src/lib/components/InputGroup/InputGroup.jsx Show resolved Hide resolved
src/lib/components/InputGroup/InputGroup.jsx Show resolved Hide resolved
src/lib/components/InputGroup/README.mdx Outdated Show resolved Hide resolved
src/lib/components/InputGroup/README.mdx Show resolved Hide resolved
@adamkudrna
Copy link
Member

Please do a rebase against the latest master, thank you!

adamkudrna
adamkudrna requested changes Mar 1, 2023
View reviewed changes
Copy link
Member

@adamkudrna adamkudrna left a comment

Choose a reason for hiding this comment

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

When I was implementing the support of custom inner border radius, I found a few more details that need to be changed. Will you please have a look at it? 🙏🏻

src/lib/components/Button/Button.jsx Outdated Show resolved Hide resolved
src/lib/components/InputGroup/InputGroup.jsx Show resolved Hide resolved
@adamkudrna
Copy link
Member

Theming of inner border radius: #454.

adamkudrna
adamkudrna requested changes Apr 26, 2023
View reviewed changes
Copy link
Member

@adamkudrna adamkudrna left a comment

Choose a reason for hiding this comment

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

Cool! 👍🏻 Last two major things before ✅:

  1. The context of horizontal form layout needs to be fixed.
  2. I'd push harder in documenting all the exceptions of when component's props are overridden or suppresed by parent InputGroup.

src/lib/components/InputGroup/README.mdx Outdated Show resolved Hide resolved
src/lib/components/InputGroup/README.mdx Show resolved Hide resolved
src/lib/components/SelectField/SelectField.jsx Show resolved Hide resolved
@adamkudrna adamkudrna force-pushed the feature/430 branch 2 times, most recently from 7be906e to b5fc344 Compare April 29, 2023 15:21
adamkudrna
adamkudrna approved these changes May 3, 2023
View reviewed changes
Copy link
Member

@adamkudrna adamkudrna left a comment

Choose a reason for hiding this comment

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

Now I believe it's all done 🙂.

@adamkudrna adamkudrna merged commit 11c3fd8 into master May 3, 2023
@adamkudrna adamkudrna deleted the feature/430 branch May 3, 2023 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbohal mbohal mbohal requested changes

@bedrich-schindler bedrich-schindler bedrich-schindler approved these changes

@adamkudrna adamkudrna adamkudrna approved these changes

@hubacekj hubacekj Awaiting requested review from hubacekj

Assignees

@dacerondrej dacerondrej

Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

create inputgroup component
4 participants
@dacerondrej @adamkudrna @mbohal @bedrich-schindler