Skip to content

Move close icon to components in docs#28512

Merged
MartijnCuppens merged 2 commits intomasterfrom
master-mc-move-close-icon
May 10, 2019
Merged

Move close icon to components in docs#28512
MartijnCuppens merged 2 commits intomasterfrom
master-mc-move-close-icon

Conversation

@MartijnCuppens
Copy link
Member

The close icon is a component, not a utility.

@XhmikosR
Copy link
Member

Should we backport this?

@MartijnCuppens
Copy link
Member Author

Nah, it's just the place where it's documented, not that important but if we're going to use the #28445 for utilities, it would be better if we move this.

@XhmikosR
Copy link
Member

@mdo: your thoughts?

@ysds
Copy link
Contributor

ysds commented May 10, 2019

The close icon is a component

My only concern is:

float: right;

If remove this, the close button is going to be a truly component.

@MartijnCuppens MartijnCuppens force-pushed the master-mc-move-close-icon branch from a111b8e to f2d1ed2 Compare May 10, 2019 16:55
@MartijnCuppens MartijnCuppens requested a review from a team as a code owner May 10, 2019 16:55
@MartijnCuppens
Copy link
Member Author

.close is only used by .modal (positioned with flexbox) and .alert (positioned with position: absolute) so we can easily drop float: right (which I just did)

@ysds
Copy link
Contributor

ysds commented May 10, 2019

LGTM👍, but would be perfect by changing its name to "Close button" IMO 😄

@MartijnCuppens MartijnCuppens merged commit 5db281d into master May 10, 2019
@MartijnCuppens MartijnCuppens deleted the master-mc-move-close-icon branch May 10, 2019 18:27
@MartijnCuppens
Copy link
Member Author

It could also be a link in another context, so let's leave it for now.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants