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

Fix input group border radii #31953

Merged
merged 5 commits into from Oct 26, 2020
Merged

Fix input group border radii #31953

merged 5 commits into from Oct 26, 2020

Conversation

MartijnCuppens
Copy link
Member

@MartijnCuppens MartijnCuppens commented Oct 22, 2020 •
edited

@MartijnCuppens MartijnCuppens requested a review from a team as a code owner October 22, 2020 09:39
@MartijnCuppens MartijnCuppens force-pushed the main-mc-input-group-border-radii branch 2 times, most recently from dde1a3a to 91b4c2c Compare October 22, 2020 09:47
@MartijnCuppens MartijnCuppens marked this pull request as draft October 22, 2020 09:51
@MartijnCuppens MartijnCuppens changed the title Main mc input group border radii Fix input group border radii Oct 22, 2020
mdo
mdo approved these changes Oct 22, 2020
View reviewed changes
Copy link
Member

@mdo mdo left a comment

Choose a reason for hiding this comment

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

Damn, what a doozy of a PR. Props for a fairly elegant solution here.

@mdo
Copy link
Member

mdo commented Oct 22, 2020

This could be backported, yeah? Technically not a breaking change if we added to v4.6.0 since we're adding a class that fixes a longstanding bug.

@MartijnCuppens
Copy link
Member Author

Hmm, this is still not the solution we wanted, there are still some issues like these:
image

I think we'll need a JS solution so that we can move the validation messages out of the input-group.

@MartijnCuppens MartijnCuppens marked this pull request as ready for review October 22, 2020 18:30
@MartijnCuppens
Copy link
Member Author

Ok, nevermind, I think I nailed it now 😄

This could be backported, yeah? Technically not a breaking change if we added to v4.6.0 since we're adding a class that fixes a longstanding bug.

Indeed, we're adding a class to fix the issue. That's not a breaking change since it was already broken 😛

@ffoodd
Copy link
Member

ffoodd commented Oct 23, 2020

LGTM! Shouldn't this be added to the migration guide? Or backporting it to v4 means this is not needed?

@XhmikosR XhmikosR added the v4 label Oct 23, 2020
@XhmikosR XhmikosR added this to Inbox in v5.0.0-alpha3 via automation Oct 23, 2020
@XhmikosR XhmikosR added this to Inbox in v4.6.0 via automation Oct 23, 2020
@mdo
Copy link
Member

mdo commented Oct 23, 2020

LGTM! Shouldn't this be added to the migration guide? Or backporting it to v4 means this is not needed?

I think it'd be helpful to mention in Migration because people want this to be a bugfix or breaking change.

@MartijnCuppens MartijnCuppens mentioned this pull request Oct 24, 2020
Merged
@MartijnCuppens
Copy link
Member Author

Updated the migration guide and opened #31962 to backport this to v4

@mdo mdo moved this from Inbox to Review in v5.0.0-alpha3 Oct 26, 2020
@mdo mdo moved this from Review to Approved in v5.0.0-alpha3 Oct 26, 2020
@mdo mdo mentioned this pull request Oct 26, 2020
Closed
@mdo mdo merged commit 6df0cfe into main Oct 26, 2020
v5.0.0-alpha3 automation moved this from Approved to Shipped Oct 26, 2020
@mdo mdo deleted the main-mc-input-group-border-radii branch October 26, 2020 22:23
@XhmikosR XhmikosR removed this from Inbox in v4.6.0 Oct 27, 2020
@XhmikosR XhmikosR mentioned this pull request Nov 14, 2020
Closed
@mhw mhw mentioned this pull request Apr 6, 2021
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ffoodd ffoodd ffoodd approved these changes

@mdo mdo Awaiting requested review from mdo

Assignees
No one assigned
Labels
backport-to-v4 css v4 v5
Projects
No open projects
v5.0.0-alpha3
  
Shipped
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Missing border radius on input group with validation feedback
4 participants
@MartijnCuppens @mdo @ffoodd @XhmikosR