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 menu style for the sidebar #198

Closed
wants to merge 1 commit into from
Closed

Fix menu style for the sidebar #198

wants to merge 1 commit into from

Conversation

mgrachev
Copy link
Contributor

@mgrachev mgrachev commented Nov 9, 2015

Hello!

In the sidebar for the last item in the list, there is no retreat from the bottom. Because of this, the last item in the list is sometimes not visible:

before

I modified styles to fix it:

after

@@ -4,11 +4,15 @@
overflow-y: auto;
padding: 0 $base-spacing;

&__list li:first-child a {

Choose a reason for hiding this comment

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

Selector should have depth of applicability no greater than 2, but was 3

@c-lliope
Copy link
Contributor

c-lliope commented Nov 9, 2015

@mgrachev thanks!

One thing I'm concerned about is that this only solves the problem for the specific number of models that you have in your app. If you had one more model, it would still fall off the end.

Have you tried scrolling in the sidebar? It scrolls separately from the rest of the page's content, so you should be able to access all of your models even if you have a bunch. Let me know if that's not working.

@mgrachev
Copy link
Contributor Author

mgrachev commented Nov 9, 2015

Scroll to the end of the sidebar does not help, status bar hides "DotaPlayer" - it can be seen in the screenshot:

scroll

@zenati
Copy link

zenati commented Nov 9, 2015

👍

@c-lliope c-lliope added the ready label Nov 12, 2015
@c-lliope c-lliope added this to the v0.1.2 milestone Nov 12, 2015
@c-lliope
Copy link
Contributor

c-lliope commented Dec 9, 2015

Merged in as 1bf79d1. Thanks!

@c-lliope c-lliope closed this Dec 9, 2015
@c-lliope c-lliope removed the ready label Dec 9, 2015
c-lliope added a commit that referenced this pull request Dec 9, 2015
Changes:

```
* [#251] [FEATURE] Raise a helpful error when an attribute is missing from
  `ATTRIBUTE_TYPES`
* [#298] [FEATURE] Support ActiveRecord model I18n translations
* [#312] [FEATURE] Add a `nil` option to `belongs_to` form fields
* [#231] [UI] Fix layout issue on show page where a long label next to an empty
  value would cause following fields on the page to be mis-aligned.
* [#309] [UI] Fix layout issue in datetime pickers where months and years
  would not wrap correctly.
* [#306] [UI] Wrap long text lines (on word breaks) on show pages
* [#214] [UI] Improve header layout when there is a long page title
* [#198] [UI] Improve spacing around bottom link in sidebar
* [#206] [UI] Left-align checkboxes in boolean form fields
* [#315] [UI] Remove the `IDS` suffix for `HasMany` form field labels
* [#259] [BUGFIX] Make installation generator more robust
  by ignoring dynamically generated, unnamed models
* [#243] [BUGFIX] Fix up a "Show" button on the edit page that was not using the
  `display_resource` method.
* [#248] [BUGFIX] Improve polymorphic relationship's dashboard class detection.
* [#247] [BUGFIX] Populate `has_many` and `belongs_to` select boxes
  with the current value of the relationship.
* [#217] [I18n] Dutch
* [#263] [I18n] Swedish
* [#272] [I18n] Danish
* [#270] [I18n] Don't apologize about missing relationship support.
* [#237] [I18n] Fix broken paths for several I18n files (de, es, fr, pt-BR, vi).
* [#266] [OPTIM] Save a few database queries by using cached counts
```
c-lliope added a commit that referenced this pull request Jan 14, 2016
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.
@c-lliope c-lliope mentioned this pull request Jan 14, 2016
c-lliope added a commit that referenced this pull request Jan 15, 2016
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.
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.

4 participants