Skip to content

Fix input group border radii#31953

Merged
mdo merged 5 commits intomainfrom
main-mc-input-group-border-radii
Oct 26, 2020
Merged

Fix input group border radii#31953
mdo merged 5 commits intomainfrom
main-mc-input-group-border-radii

Conversation

@MartijnCuppens
Copy link
Member

@MartijnCuppens MartijnCuppens commented Oct 22, 2020 •
edited
Loading

@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 MartijnCuppens requested a review from mdo October 22, 2020 18:02
@MartijnCuppens
Copy link
Member Author

MartijnCuppens commented Oct 22, 2020

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

MartijnCuppens commented Oct 22, 2020

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
@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 force-pushed the main-mc-input-group-border-radii branch from 11497f9 to 07483d8 Compare October 24, 2020 16:47
@MartijnCuppens MartijnCuppens mentioned this pull request Oct 24, 2020
Merged
@MartijnCuppens
Copy link
Member Author

MartijnCuppens commented Oct 24, 2020

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

@mdo mdo mentioned this pull request Oct 26, 2020
Closed
@mdo mdo merged commit 6df0cfe into main Oct 26, 2020
@mdo mdo deleted the main-mc-input-group-border-radii branch October 26, 2020 22:23
@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

None yet

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