w3c / rdf-semantics

https://w3c.github.io/rdf-semantics/
Other
6 stars 2 forks source link

improve display on mobile phones #30

Open domel opened 1 year ago

domel commented 1 year ago

Preview | Diff

pfps commented 1 year ago

There are lots of changes that appear to be formatting. They should be separated out into a separate PR.

pfps commented 1 year ago

What does this actually do?

TallTed commented 1 year ago

@pfps — I see only CSS changes — whitespace on several lines (looks like no effect on the stylesheet), and addition of one line (99):

table { word-break: break-all; max-width: 80vw; }

I think that should be break-word instead of break-all, as discussed in another PR, and have no problem with the whitespace cleanup.

@gkellogg — How are you seeing those "dn inside dfn" issues?

TallTed commented 1 year ago

@gkellogg's concerns came from the Echidna Auto-publish failure report, to wit:

  "file:/home/runner/work/rdf-semantics/rdf-semantics.w3c/spec/index.html":1535.7-1535.130: error: The element “dfn” must not appear as a descendant of the “dfn” element.
    "file:/home/runner/work/rdf-semantics/rdf-semantics.w3c/spec/index.html":1535.131-1535.285: error: Start tag “a” seen but an element of the same type was already open.
    "file:/home/runner/work/rdf-semantics/rdf-semantics.w3c/spec/index.html":1535.131-1535.285: error: End tag “a” violates nesting rules.
    "file:/home/runner/work/rdf-semantics/rdf-semantics.w3c/spec/index.html":1536.7-1536.103: error: The element “dfn” must not appear as a descendant of the “dfn” element.
    "file:/home/runner/work/rdf-semantics/rdf-semantics.w3c/spec/index.html":1536.104-1536.233: error: Start tag “a” seen but an element of the same type was already open.
    "file:/home/runner/work/rdf-semantics/rdf-semantics.w3c/spec/index.html":1536.104-1536.233: error: End tag “a” violates nesting rules.
    "file:/home/runner/work/rdf-semantics/rdf-semantics.w3c/spec/index.html":1536.269-1536.272: error: Stray end tag “a”.
    "file:/home/runner/work/rdf-semantics/rdf-semantics.w3c/spec/index.html":1536.269-1536.278: error: Stray end tag “dfn”.
    "file:/home/runner/work/rdf-semantics/rdf-semantics.w3c/spec/index.html":1536.269-1536.282: error: Stray end tag “a”.
    "file:/home/runner/work/rdf-semantics/rdf-semantics.w3c/spec/index.html":1536.269-1536.288: error: Stray end tag “dfn”.

Now, those errors appear to be about lines 1535 and 1536 —


  <p>Both of these can be handled by allowing the rules to apply to a generalization of the RDF syntax

— which clearly do not contain the tags flagged by Echidna.

Hopefully, once my change request is merged, which will re-trigger Echidna and PR-Preview, the issues will magically resolve, or the errors will be revised and point to lines that actually contain problematic tags.

pfps commented 1 year ago

These may all be the result of malformed XML around line 202. I'll put in a PR to try to fix the problem.

See #31

pfps commented 1 year ago

I'm leaving the needs discussion label, not because there hasn't been discussion but because the final version of the PR needs careful consideration.

domel commented 1 year ago

According to telecon and different suggestions, I've just changed CSS for the tables here. If you accept this layout for small devices, I will implement these changes in other specs.

domel commented 1 year ago

I have prepared other PRs (some of them I improved more if needed). So I'm waiting for 'yes'.

pfps commented 1 year ago

I thought the changes were going to be conditioned on the type of device. Why is this not happening?

It appears that the width of tables are being restricted to 80% of something. This seems wrong to me. Why not allow tables to be as wide as that thing?

This document is not a very good one for testing out the changes as there are only a few tables with more than one column and none of them are very wide.

TallTed commented 1 year ago

Looks far better to me, overall. I see breaks are not happening mid-word. This does mean that some side-scrolling will be necessary in some smaller-windowed environments. Echidna errors appear to have been resolved.

@pfps — I'm guessing that "It appears that the width of tables are being restricted to 80% of something" because you see table { word-break: normal; overflow-wrap: anywhere; max-width: 80vw; }, where the 80vw does indeed limit the table width to 80% of the viewport, which is the browser window. However, if you have large viewports available, you should see that this limit is subordinate to other limitations, so doesn't actually come into play. If you have small viewports available, you should see that this limit doesn't get enforced because the unbroken words cause the table cells to be wider than this setting, to contain those unbroken words.

pfps commented 1 year ago

As far as I can tell, the 80vw is limiting the width of tables in some places and, again as far as I can tell, the limit is active. For example, in the PR preview for some window widths the RDFS Semantic Conditions table is narrower than the text width and some lines overflow when at the same width they do not overflow without the PR.

So, again, why limit table width?

domel commented 1 year ago

Because 80vw is the default width in respec for the content. Note that 10vw are left and right margin.

pfps commented 1 year ago

This does not match with what I see. With the PR the width of the RDFS Semantic Conditions table is narrower than without the PR. So if there is a change, how can 80vw be the default?

In any case, if 80vw is the default then there should be no reason to include it (as it would currently be doing nothing) and a good reason to not include it (as it would override future changes in RESPEC).

TallTed commented 1 year ago

if 80vw is the default then there should be no reason to include it

I wonder if it wouldn't be worth creating a (or a few) separate PR for testing, perhaps made against one of our documents with more tables, especially tables that are problematic before whatever CSS tweak we find to be best?

TallTed commented 1 year ago

@pfps — If you could provide links (including fragment ID) to the table(s) you're seeing as problematic, that will help me (and, I imagine, others) to dig into possible solutions.

pfps commented 1 year ago

As stated above, it's the RDFS semantic conditions table. As far as I can tell there is no direct link to it. I've always either scrolled through the document or searched for the caption.

TallTed commented 1 year ago

@pfps — Do you mean the table immediately above this anchor? (which should indeed have its own ID! as I've added in PR#33), that has only one column?

Or the table following this anchor, now also at this direct link from PR#33 that has 3 columns of odd widths (left and right columns have odd wraps, and center could lose a bit of width to eliminate those wraps)...?

pfps commented 1 year ago

There is only one table for RDFS semantic conditions - the one with RDFS semantic conditions as its caption - the first one. The other is RDFS entailment patterns.

pfps commented 1 year ago

The other table is actually damaged in the PR when I look at it. In some of the LH columns the rule name is split, e.g., rdfs4 is on one line and a is on the next. This is not the case without the PR.

I thought that there was not going to be any word splitting going on.

gkellogg commented 1 year ago

I’ve noted similar issues in Turtle and TriG, where tables view properly until the width is reduced enough, and then they shrink to narrower than surrounding content.

domel commented 1 year ago

Could you write more details? In what size of screen did you see that effect?

gkellogg commented 1 year ago

I simply narrowed the window I was looking at in Safari on my Mac, when it got down to around t 700-900 pixels wide, the table became narrower than the surrounding text. Before, it was the same width.

domel commented 1 year ago

OK, I will fix that. But remember that the overwhelming majority of mobile devices are covered by my patch. I tested it on 46 mobile devices. But ok, I'll try to work it out a bit more ( I mean in all PRs about RWD).

pfps commented 1 year ago

It's not testing on mobile devices that is required, but testing on non-mobile devices.

A requirement for this PR is that it have no negative effects on non-mobile devices.

TallTed commented 1 year ago

Welcome to HTML+CSS hell. Things that shouldn't happen, do. Things that should happen, don't.

The table with the "RDFS semantic conditions" caption has only one column, so it's odd that it's a table at all. That said, I see nothing problematic with it. The 80vw setting is a maximum width, not an always width, so it being slightly smaller than that max (as I see in all viewports that are sufficiently large), which is wide enough that no words split.

In the other table, the unfortunate break/wraps of the first column occur when the value (here, rdfs1) is wrapped by spaces, as —

<dfn id="dfn-rdfs1" tabindex="0" aria-haspopup="dialog" data-dfn-type="dfn"> rdfs1 </dfn>

— but do not occur when the value is not wrapped by spaces, as —

<dfn id="dfn-rdfs1" tabindex="0" aria-haspopup="dialog" data-dfn-type="dfn">rdfs1</dfn>

(I'm only viewing this is current Chrome 113.0.5672.92 (Official Build) (x86_64) on macOS Mojave 10.14.6 (18G9323), at the moment. If there's a known problematic browser, I can try viewing and analyzing there, too.)

gkellogg commented 1 year ago

I still don't understand why the table in 9.2.1 Patterns of RDFS entailment is the full-width of the text area until the window is shrunk sufficiently, then it is narrower than the text width. It's better than before, and perhaps good enough.

domel commented 1 year ago

@gkellogg If I understand you correctly, it depends on the content. The table takes up less than the entire webpage (on small screens) because all lines fit and it does not need to expand anymore.

pfps commented 1 year ago

Have all the deleterious effect of previous versions of the PR been eliminated?

domel commented 1 year ago

@pfps Yes, it seems to me that all deleterious effects (although I wouldn't call it that) have been corrected. This applies to all other PRs, which differ from each other because their structure is different, but the effect is the same.

domel commented 1 year ago

@pfps

I see even worse behaviour now.

It's your subjective feeling.

Not only does the RDFS Semantic Conditions table (the yellow table in https://pr-preview.s3.amazonaws.com/w3c/rdf-semantics/pull/30.html#rdfs_interpretations) shrink horizontally more than regular lines, but now the font shrinks.

Yes exactly. This is how it solves the issue of small screens, small screens need a smaller font because they have a smaller surface area. By the way, this effect has been around for a long time.

Please completely vet the changes before asking for yet another review.

I checked the changes carefully. If you don't like them, I invite you to change the HTML and CSS. I will not change anything because I believe that this is the best possible solution, many times better than the original (before PR).

pfps commented 1 year ago

No, these are objective findings.

Having table width constrained to be less than line width and having that cause the table to consume more vertical space is an objective deleterious effect.

Having the font size of a table get smaller when there is still room on the right-hand margin is an objective deleterious effect.

domel commented 1 year ago

As I said above, I disagree. Note that each table has a different width, so the effect you're imagining would require you to write separate styles for each table. And in the future, anything you add to these tables will likely make the table too small to fit on smaller screens again. In short, I do not want and will not change anything. As I understand it, you can continue working on your own, or go back to the original version, which renders much worse on small screens. BTW, those yellow areas shouldn't be tables at all, that's against the sematics of HTML elements.

pfps commented 1 year ago

No. There is no reason whatsoever to leave blank space on the right-hand side of tables when regular lines are longer and the table font is reduced or lines are unnecessarily split. This is not a matter of the natural width of tables - many tables have their width unnecessarily reduced and this reduction has deleterious effects.

domel commented 1 year ago

Okay, so I'm waiting for your code and let me comment on it.

pfps commented 1 year ago

I am not the one that wants changes. I view the current situation as much better than any of the PRs that have been proposed. I am happy leaving things as they are.

domel commented 1 year ago

If you think that on mobile phones the original version displays better than the current one, then I can't talk to you. I don't think there's any point in wasting any more of your time or mine. However, I advise you to take a few mobile phones, check and rethink the situation. One more question I would ask myself if I were you is which specific devices (and what percentage of all) show the defect you are talking about (because manually resizing the desktop browser is not a determinant here).

pfps commented 1 year ago

My investigations have only been on my laptop. I have never implied anything about behaviour on mobile phones. All I have ever stated is that every version of the PR that I have looked at is significantly worse viewed on my laptop than the status without the PR.

My view is that the PR is unacceptable if it makes things worse when viewing the document on a laptop and for every version of the PR this has been the case.

domel commented 1 year ago

Could you show me a screenshot from your laptop? And even better if you mark the areas in the screenshot that are problematic. I tried to check on most laptop sizes, but maybe yours is rare enough that I missed it.

pfps commented 1 year ago

Here are two screenshots, one with PR 30 and one without. In the one with, the width of the table has been unnecessarily reduced and the font size also unnecessarily reduced. This only happens in some window widths.

Screenshot from 2023-05-16 18-41-27 Screenshot from 2023-05-16 18-42-06

Many other tables have similar problems.

pfps commented 1 year ago

@gkellogg I run with small fonts (relative to screen distance) so further shrinkage is bad for me. At some point it may be better to shrink fonts but the breakpoint is much too high for me with this PR.

This is the first subjective issue that I have reported and even then there is an objective component - unnecessary shrinkage of fonts is objectively bad.

domel commented 1 year ago

@pfps You can now see if you like it on your super small computer monitor.

pfps commented 1 year ago

My laptop is a Yoga C940 with a 14" screen. I suppose that this may seem super small compared to the gigantic phones and phablets that are in vogue now, but I find it just the right size for me.

In any case, as far as I can tell it is not the size of the screen that matters, but how wide the browser window is. It is quite common to have multiple windows on screens with them dividing up the the horizontal space somehow. This makes it routine for the width of a window to be quite small even on large screens.

domel commented 1 year ago

@pfps OK, nevermind. Let's get to the specifics. Are you satisfied with the current commit?

pfps commented 1 year ago

Here are two screenshots that show an objective degredation with the current PR. When the window reaches a certain width the font size in table is immediately reduced with no reference to any benefit there might be to overcome this degredation. For both tables in these screenshots there is zero benefit.

Screenshot from 2023-05-17 18-13-53 Screenshot from 2023-05-17 18-15-27

For me, this changes the table from being easily readable to not being easily readable.

pfps commented 1 year ago

A separate question is whether this kind of font hackery is ever justified. I'm not an expert in user interface design to determine this, but some expert should probably be consulted.

domel commented 1 year ago

For me, this changes the table from being easily readable to not being easily readable.

It's depend on how you define "readable". It's definitely more readable in mobile phones and tablets because of no need to scrolling. It's the same readable in normal laptops and desktops (the font size is the same). We probably won't please everyone, but the solution currently covers as many of the cases as possible. As I wrote earlier, I have been professionally involved in front-end development for a long time and if you feel the need to check with another expert, I have no fear that you will hear the same.

domel commented 1 year ago

Back to the font sizes themselves. I'm shocked that you don't mind the font size in the pink and green areas and in the yellow areas the same font size is a problem for you.

TallTed commented 1 year ago

It's hard to see the difference between those screenshots, with them stacked vertically. This may be helpful to others:

wider, has bigger font in the tables narrower, has smaller font in the tables

I agree with @pfps that this makes the tabular data harder to read, even at the substantially blown-up size seen in these screenshots, vs the font sizes seen in the actual document.

domel commented 1 year ago

@TallTed OK, I propose to accept the PR (which is about CSS) and open a new one to refractor HTML. Yellow, pink, etc areas are the tables, and they shouldn't be (in HTML a table element is for tabular data and it is not for tabular data). So I propose to change HTML tables there to block elements (that should look like the normal text including font size).

TallTed commented 1 year ago

@domel — I agree that the pictured tables shouldn't be tables. Replacing these with block elements seems reasonable, presuming the background and text color effects are preserved.

That said, I think that the font size changes observed by @pfps are likely CSS-related, so I think @pfps should look at the actually tabular data in the other tables, and see whether they do the same font-size-shrinking with the same action — as if they do, this issue #30 should remain open and the CSS driving that should be addressed.