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 UI regressions #408

Merged
merged 2 commits into from
Jan 15, 2016
Merged

Fix UI regressions #408

merged 2 commits into from
Jan 15, 2016

Conversation

c-lliope
Copy link
Contributor

When merging in #198 and #214,
the Percy.io visual regression tests weren't run
because our quota for the month had been used up.

Without the visual regression tests,
we failed to notice two visual regressions that were introduced.

The two commits in this PR fix the visual regressions that were introduced.
See the commit descriptions for more information.

@c-lliope
Copy link
Contributor Author

Unfortunately... we've already run through our Percy quota for this month, so we can't automatically verify that these fixes are valid.

@@ -3,6 +3,7 @@
max-width: 12em;
overflow-y: auto;
padding: 0 $base-spacing;
padding-bottom: $base-spacing;
Copy link
Contributor

Choose a reason for hiding this comment

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

This addition means we are defining padding twice. Line 5 applies 0 to the padding-bottom, and Line 6 we overwrite that.

Instead, we can just update Line 5 to; padding: 0 $base-spacing $base-spacing;

@tysongach
Copy link
Contributor

One minor comment, otherwise 👍

Problem:

When merging in #198,
the Percy.io visual regression tests weren't run
because our quota for the month had been used up.

Because we did not have the assurance of visual regression tests,
we didn't realize that the `:last-child` selector
that we applied to sidebar links
actually applied to all of the sidebar links, because each was wrapped
in their own `<li>` parent.

This increased the spacing between the links above what we wanted.

Solution:

Move the padding from the last link onto its parent element,
the sidebar `<ul>` element.
Problem:

When merging in #214,
the Percy.io visual regression tests weren't run
because our quota for the month had been used up.

In that PR, we re-aligned the button in the header.
This change accidentally affected content below the header.

Solution:

Change the alignment from `baseline` to `flex-start`.
This keeps the button from affecting content below the header.
@c-lliope c-lliope closed this Jan 15, 2016
@c-lliope c-lliope deleted the gw-ui-fixes branch January 15, 2016 21:58
@c-lliope c-lliope merged commit cce7d6c into master Jan 15, 2016
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.

2 participants