Fix table scroll on mobile

[zemzelett]

Hey there,

i'm using the RGBDS docs quite often, saw the list of issues and thought i might have a go on some of those.

This pull request addresses issue #10 regarding the tables overflowing and causing scrolling which messes with the general layout.

I chose to conditionally remove the indentation of the tables on mobile devices (<480px) and use word-break: break-word in order to get the table fitted within the available screen space. In order to have sane column widths, i set those to have a minimum of 15vw.

I checked all pages and think this might be a solution to the problem stated.

[ISSOtm]

Please be aware that word-break: break-word; is deprecated. The recommended equivalent should be used instead.

Given how much the words are being broken, I think this solution is a step in the right direction, but not quite mergeable yet. First, how wide is the viewport in the example you showcased? It seems really narrow, even for a phone screen. Depending on the minimum, we may have to let lines overflow regardless when really too tight, and allow some horizontal scrolling, for example.

I can also see that 2822e9b broke the navbar collapsing breakpoint >_> I ought to fix that, and perhaps make a note for such changes in the future.

[zemzelett]

Wow, i've never noticed the deprecation note on break-word, thanks for the hint. I've looked a bit into the alternatives. I've found nothing that achieves, what i wanted to achieve yet.

For your question, the viewport of the device is 280x653 (Galax Fold in Google Chrome). It's the smallest device i could find. The break-point i chose is a width of 480px, which is a common one that target phones. On screen widths above 300px the word-break situations looked fine, for my taste, that is.

Allowing horizontal scrolling would be an option. But from experience i'd advise that one limits table widths to something sane as horizontal scrolling on a phone is a very undesirable experience.

With aggressive you probably mean, that even the names, like __ISO_8601_LOCAL__, are broken into pieces? If that is not desired, we have to necessarily find a solution with scrolling. I would then suggest a max width for the tables (e.g. table-layout: fixed; max-width: 200vw).

From what i see now, i'd look into a mix of some horizontal scrolling like proposed in the last paragraph and then breaking text on word boundaries. Something like this:

There's no breaking of words themselves (first and second column), so tables might expand beyond the viewport. If sentences can be broken within a table cell (third column), they will be though. What do you think?

[ISSOtm]

Yeah, that does look good! The text in the third column is also broken into acceptable lengths as well.

[zemzelett]

I just pushed the necessary changes for the effect shown. Plus some additions to accompany them.

Those are:

• The navbar was one pixel too large and caused a hardly visible thinner line at the top when scrolling vertically and horizontally simultaneously.

  

  vs.

  
  If the line is supposed to be the thinner line all the way through, that's easily doable too but invovles more changes and decision to be made.

• The body had the tiled background image showing up when scrolling horizontally, too prevent that i set a solid background color on screen sizes <480px

[ISSOtm]

• The navbar was one pixel too large and caused a hardly visible thinner line at the top when scrolling vertically and horizontally simultaneously.

There are two hard things in programming... :)

If the line is supposed to be the thinner line all the way through, that's easily doable too but invovles more changes and decision to be made.

Given its nature, I believe this is a mistake. It's better changed, then.

• The body had the tiled background image showing up when scrolling horizontally, too prevent that i set a solid background color on screen sizes <480px

See the review comments below.

[ISSOtm]

Shouldn't this apply to the rest of the website as well? Additionally, I'm not sure about overriding the background color like this and not either
a) using a variable to avoid duplicating the background color, or
b) using unset (?) on the background-image property (if possible).

[zemzelett]

What do you mean by "the rest of the website"? Like in general, disregarding the device it's viewed on? Then the background image would vanish completely and we could be thinking of removing its definition entirely.
I put it only for phones because the background-image is never visible there anyways (it only shows up on screen larger then tablets).

The background: unset would have to happen on the html element, because the definition for the background is on it. An unset would lead to a white background though! So we'd have to go with choice a).

[ISSOtm]

I mean applying this style rule to the whole website instead of just the docs.

[zemzelett]

Oh, now i see. Sorry. Ok, will do.

[ISSOtm]

I'd like the breakpoint to be a variable, so that it can be used consistently in any future changes.

[zemzelett]

Noted, will do.

[ISSOtm]

Maybe 50px + $navbar-border-bottom? Or maybe that variable can be added to the height in the relevant rule... or maybe we should use a different box-sizing? I'm not sure which is the more sensible option.

[zemzelett]

Any even number leads to that effect. (So 49, 51 are fine - the line is a thick as defined, but 48, 50 or 52 are not and show that sligthly larger border).

I tried using a margin-top: 1px on the scroller, but that leaves a visible gap between the navbar and the scroller, which i found to be undesireable:

box-sizing should make no difference on this issue, as the border is just visibly too large but measurements are just fine.

We could also consider this negligible. I mean, it's hardly visible anyways, so that's a fair option too.

[ISSOtm]

Fixing a blunder makes sense if it's a small change, even if it's just a 1-pixel strip. If the value must be odd, then please note as such in a comment—and make sure to use a // comment so that it's SCSS-only.

[zemzelett]

Alright. Thanks for the heads up on the //. :)