search

kni-labs / rrssb

:arrow_right_hook: Ridiculously Responsive Social Sharing Buttons
https://rrssb.netlify.com/
MIT License
3.4k stars 423 forks source link

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

Closed AdamPS closed 8 years ago

AdamPS commented 9 years ago

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 commented 9 years ago

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

AdamPS commented 9 years ago

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 commented 9 years ago

BTW my changes also include the fix for #128

AdamPS commented 9 years ago

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

AdamPS commented 9 years ago

See above pull request, many thanks!

adamschwartz commented 9 years ago

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

I made this jsFiddle for easy testing. :)

AdamPS commented 9 years ago

@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 commented 9 years ago

Sweet, thanks @AdamPS :)

eiselems commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

:+1:

dbox commented 8 years ago

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 commented 8 years ago

@dbox Nice, looking forward to it ;)