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

Raise font-size #1777

Merged
merged 2 commits into from
Mar 22, 2017
Merged

Raise font-size #1777

merged 2 commits into from
Mar 22, 2017

Conversation

tvdeyen
Copy link
Member

@tvdeyen tvdeyen commented Mar 16, 2017

All our forms are nested in field sets, which have a font-size of 90%. Also our form field font sizes are reduced by 10%. With our base font size of 13px this leads to a 10.5px font size. This is definitely too small for modern screen sizes.

Before

solidus 13px

After

orders 2017-03-19 21-49-48

@tvdeyen tvdeyen force-pushed the larger-admin-font-size branch from 7bf867d to 27fc1da Compare March 16, 2017 22:18
@tvdeyen tvdeyen self-assigned this Mar 16, 2017
@tvdeyen tvdeyen added WIP and removed needs work labels Mar 16, 2017
@jhawthorn
Copy link
Contributor

jhawthorn commented Mar 16, 2017

If we're tweaking font sizes, I think we should remove some strange font size rules. Otherwise we risk calibrating the base font size wrong because the elements we look at are at font-size: 90%

For example:

Fixing these should make the fonts even larger at the same time, which I think aligns with your goals 😄 .

@Mandily
Copy link
Contributor

Mandily commented Mar 16, 2017

I would love to see a plan to deal with text that doesn't fit. I suspect if this happens at all it would be in the table headers. I'd be ok with truncating it like we do the admin email in the left hand column until we get a better layout sorted out.

@tvdeyen
Copy link
Member Author

tvdeyen commented Mar 16, 2017

Fixing these should make the fonts even larger at the same time, which I think aligns with your goals 😄 .

👍

truncating it like we do the admin email in the left hand column

I don't think it's a good idea to truncate table headers. IMO they transport important information. But I may be wrong. I would like to widen the default container width from 960px to ~1080px instead.

@tvdeyen
Copy link
Member Author

tvdeyen commented Mar 16, 2017

Most ideally we would use the newly introduced full width layout everywhere, but this would cause adjustments to the search forms most of the tables have. Maybe introduce a max width container for the search form?

@Mandily
Copy link
Contributor

Mandily commented Mar 17, 2017

The full width layout is for the listing pages with tables, but if there was a small table in a mostly form page I had hypothesized that text may not fit as shown in the example below. In the case that the text overflowed the table header that it was in, I thought it better to truncate it. Now thinking about it further, the best solution would be to simple wrap the text. :)

screen shot 2017-03-17 at 9 24 52 am

@tvdeyen
Copy link
Member Author

tvdeyen commented Mar 17, 2017

This could work. Alternatively we could remove columns that hold secondary information on smaller viewports. Hard to figure out which one is the secondary information though.

@tvdeyen
Copy link
Member Author

tvdeyen commented Mar 17, 2017

@Mandily First step taken #1782

@tvdeyen tvdeyen mentioned this pull request Mar 19, 2017
@tvdeyen tvdeyen force-pushed the larger-admin-font-size branch from 27fc1da to d96d113 Compare March 19, 2017 20:42
@tvdeyen tvdeyen changed the title Raise default admin body-font-size to 15px Raise font-size Mar 19, 2017
@tvdeyen tvdeyen removed their assignment Mar 19, 2017
@tvdeyen tvdeyen removed the WIP label Mar 19, 2017
@tvdeyen tvdeyen requested a review from jhawthorn March 19, 2017 20:51
@tvdeyen
Copy link
Member Author

tvdeyen commented Mar 19, 2017

@Mandily @jhawthorn I removed the change of the body font size and fixed the sizing in fieldset and the navigation instead (see updated comment on top). Thanks for pointing me to this, John!

tvdeyen added 2 commits March 19, 2017 21:55
All fieldsets (and we use lots of them) have a font size of 90% (11.3px). With our base font size of 13px this leads to very small font sizes (10.53px) in all forms inputs (that have an additional 90% font-size) as most of them sit in a fieldset.

Also removes a lot of styles already defined by Bootstrap and styles that needed to be resetetted as the fieldset was defined as part of the default input field styles.
The font size in the admin main navigation was lowered to 90%, what is very small as we have a base font size of 13px.
@tvdeyen tvdeyen force-pushed the larger-admin-font-size branch from d96d113 to bae3ad6 Compare March 19, 2017 20:56
@tvdeyen tvdeyen added the changelog:solidus_backend Changes to the solidus_backend gem label Mar 19, 2017
@Mandily
Copy link
Contributor

Mandily commented Mar 20, 2017

👍

@jhawthorn
Copy link
Contributor

This is looking much better.

@tvdeyen was there supposed to be a change to $body-font-size as well? Currently our legacy styles are using 13px and bootstrap is specifying 15px

@tvdeyen
Copy link
Member Author

tvdeyen commented Mar 22, 2017

@jhawthorn yes, but before we do that we want to do some preparations with #1780 and #1786

Otherwise raising the font size just won't look good.

@jhawthorn jhawthorn merged commit 4e66f02 into solidusio:master Mar 22, 2017
kennyadsl added a commit to nebulab/solidus that referenced this pull request Apr 6, 2017
Probably solidusio#1777 introduced some visual bugs.
This commit:

- adjusts width of the whole datepicker to adapt to its content,
  was: width: 17em;
- gives more spaces to arrows that currently are not completely shown
@kennyadsl kennyadsl mentioned this pull request Apr 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:solidus_backend Changes to the solidus_backend gem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants