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

Prevent white-space from appearing above header button when title is unusually long #214

Closed
wants to merge 1 commit into from

Conversation

murdoch
Copy link
Contributor

@murdoch murdoch commented Nov 11, 2015

When I visit the edit page for a resource with a long title I notice some unsightly white-space above the show page link that appears next to the h1 tag. See image:

flex-end

This whitespace is caused by the following CSS:

.header {
  align-items: flex-end;
}

To get rid of this white-space we can change flex-end to flex-start or baseline, see below:

baseline

I will understand if you prefer flex-start over baseline.

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

@murdoch this looks great! Can you do me a favor and rebase on master? We've added some visual regression tests to make sure CSS changes don't accidentally break things. I'm sure you're fine here, but just want to get into the habit.

After that, should be good to merge!

@c-lliope
Copy link
Contributor

Also, I think flex-start might be better here... it's a tough one.

@smharley and @devonharless, thoughts on align-items: flex-start vs align-items: baseline?

@smharley
Copy link
Contributor

baseline is lining up the bottom of the text, which you can see in the screenshot. I'd guess that flex-start would move the button up a little more. Either is probably fine, whichever you think looks better.

My only other feedback is that it would be nice to have some padding-right on that h1 so they don't bump into each other

@c-lliope
Copy link
Contributor

c-lliope commented Dec 9, 2015

Merged in as 67d8704. 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 #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 mentioned this pull request Jan 14, 2016
c-lliope added a commit that referenced this pull request Jan 15, 2016
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.
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