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

Need fix for Large-format type size when container isn't 100% width #133

Closed
AdamPS opened this issue Jul 23, 2015 · 13 comments
Closed

Need fix for Large-format type size when container isn't 100% width #133

AdamPS opened this issue Jul 23, 2015 · 13 comments

Comments

@AdamPS
Copy link
Contributor

AdamPS commented Jul 23, 2015

Almost everything is resized accurately, but the font size is a bit hit and miss. It uses vw with a fallback of pt. Why not calculate the exact best font size in the script?

pt is of course hard-coded so sometimes too big and sometimes too small
vw is relative to the viewport width - which could be very different from the container width

To see a worst possible ugly case, take the demo page, comment out all but 3 buttons, put width:25% on the buttons container and make the screen really wide. The font is way too big.

@dbox
Copy link
Contributor

dbox commented Jul 24, 2015

If only there was a cw property... container width.

@dbox dbox changed the title Font size not entirely reliable Need fix for Large-format type size when container isn't 100% width Jul 24, 2015
@AdamPS
Copy link
Contributor Author

AdamPS commented Jul 26, 2015

OK, I've got stuck in a bit and I have a fairly good solution. But I ended up simplifying the CSS quite a lot, and possibly accidentally broke something. So I would appreciate anyone who is wiling to take a look and test it out.

In summary, I got rid of all of the hard-coded px values and also the vw. Everything is now in em, except the font size of course. The js increases the font size when the screen gets wider, and everything else scales from that in the CSS by virtue of em.

As I have it by default, the icons and fonts are a bit larger than before, which is my personal preference as I felt things were a little hard to see/read before. However there are some new config settings at the top of the scss so that can put this back. $rrssb-font-size = 11px, $rrssb-button-height = 3.3, $rrssb-icon-ratio 60% is maybe more like it was before.

@AdamPS
Copy link
Contributor Author

AdamPS commented Jul 26, 2015

BTW my changes also include the fix for #128

@AdamPS
Copy link
Contributor Author

AdamPS commented Jul 26, 2015

PS The new CSS is %30 smaller (and I feel it's easier to understand)

@AdamPS
Copy link
Contributor Author

AdamPS commented Jul 26, 2015

See above pull request, many thanks!

@adamschwartz
Copy link

I’m seeing this issue as well. Happy to help out in any way I can.

I made this jsFiddle for easy testing. :)

adamschwartz added a commit to cloudflare-apps/rrssb that referenced this issue Aug 5, 2015
@AdamPS
Copy link
Contributor Author

AdamPS commented Aug 5, 2015

@adamschwartz I had it fixed in my fork, but that was based on top of other work that altered the appearance of the buttons, and so @dbox quite reasonably didn't want to take it. Unfortunately that code isn't there any more because I removed it in order to make the branch pure so another fix could be pulled.

However it was actually pretty simple. The first part you have already done I'm guessing from your above note. Remove all the CSS with vw and pt sizing based on number of buttons. The second part is a js fix. In makeExtremityBtns, search down for self.addClass('large-format');. After that, add lines something like this:
var fontSize = buttonWidth / 12 + 'px';
self.css('font-size', fontSize);
and in the else branch
self.css('font-size', '');

@adamschwartz
Copy link

Sweet, thanks @AdamPS :)

eiselems added a commit to eiselems/marcuseisele_com that referenced this issue Nov 26, 2015
@eiselems
Copy link
Contributor

@AdamPS Is this a general fix? Or would you say this is just a workaround for non 100% width containers?
If this is considered a fix, i can create a PR for my latest commit inside my fork.

@AdamPS
Copy link
Contributor Author

AdamPS commented Nov 26, 2015

I would definitely say it's a general fix - 100% width is probably not so common because of sidebars and so on. Of course need to test the fix works well at 100%, but I think it should.

@adamschwartz
Copy link

👍

@dbox
Copy link
Contributor

dbox commented Dec 8, 2015

Finally got a chance to look at this. I think this basic concept will work, we just need to tweak the sizing a little so there is no visual change.

I've also be experimenting with element queries a little, which could be interesting.

http://codepen.io/dbox/pen/adzLQb

I'll get a fix for this pushed soon.

@eiselems
Copy link
Contributor

eiselems commented Dec 8, 2015

@dbox Nice, looking forward to it ;)

@dbox dbox mentioned this issue Dec 8, 2015
dbox added a commit that referenced this issue Dec 9, 2015
@dbox dbox closed this as completed Dec 9, 2015
MagicianYang added a commit to MagicianYang/magicianyang.github.io that referenced this issue Dec 28, 2015
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

No branches or pull requests

4 participants