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

Fix styling of tiered promotions delete icon #1810

Merged
merged 1 commit into from
Mar 31, 2017

Conversation

jhawthorn
Copy link
Contributor

@jhawthorn jhawthorn commented Mar 31, 2017

Required for #1776

Previously this trash icon was being partially obscured by the row next to it, and only the left half of the icon could be clicked. It was also floating outside of the parent container. This commit fixes these issues
by adding a margin to the row, and placing the icon inside.

It also changes the cursor to a pointer on hover.

Before After

Background red showing overlap. Trash can't be clicked inside the red.

Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

just a small css formatting preference.

@@ -54,9 +54,13 @@
position: relative;
padding-bottom: 10px;

>.row {
Copy link
Member

Choose a reason for hiding this comment

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

I'd leave a space after >

Previously this trash icon was being partially obscured by the row next
to it, and only the left half of the icon could be clicked. It was also
floating outside of the parent container. This commit fixes these issues
by adding a margin to the row, and placing the icon inside.

It also changes the cursor to a pointer on hover.
@jhawthorn jhawthorn force-pushed the tiered_promotions_icon_css branch from a60eabd to 4ea4476 Compare March 31, 2017 20:26
@jhawthorn
Copy link
Contributor Author

Good call @kennyadsl. We might want to add a sass linter to catch these.

@jhawthorn jhawthorn merged commit a911de9 into solidusio:master Mar 31, 2017
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.

3 participants