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

[css-display-4] Define 'reading-order: auto' #7387 #8257

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

fantasai
Copy link
Collaborator

No description provided.

<dl dfn-type=value dfn-for=reading-order>
<dt><dfn>auto</dfn>
<dd>
Same as ''0'' except that in randomizing formatting contexts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if people may want auto || <integer> instead of restricting to 0.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an interesting idea...

css-display-4/Overview.bs Show resolved Hide resolved
css-display-4/Overview.bs Show resolved Hide resolved
css-display-4/Overview.bs Show resolved Hide resolved
css-display-4/Overview.bs Show resolved Hide resolved
then its placement in the [=reading and navigation order=] is likewise shifted.

<div class="example">
For example, [=grid items=] are laid out in a ''grid-auto-flow: column dense'' grid,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
For example, [=grid items=] are laid out in a ''grid-auto-flow: column dense'' grid,
For example, when [=grid items=] are laid out in a ''grid-auto-flow: column dense'' grid,

Comment on lines +1227 to +1228
to the latest position in the [=reading and navigation order=]
that is also after any items with ''reading-order: 0'' or ''reading-order: auto''
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"the latest position that is also after something" seems to refer to the end?
Should be "the earliest after something", or just

Suggested change
to the latest position in the [=reading and navigation order=]
that is also after any items with ''reading-order: 0'' or ''reading-order: auto''
in the [=reading and navigation order=]
after any items with ''reading-order: 0'' or ''reading-order: auto''

<dd>
Specifies which ordinal group the item belongs to.
Sibling elements are ordered starting from the lowest numbered ordinal group and going up;
elements with the same ordinal group are keep the order they appear in the source document.
Copy link
Contributor

@Loirooriol Loirooriol Jan 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
elements with the same ordinal group are keep the order they appear in the source document.
elements with the same ordinal group are kept in the order they appear in the source document.

<dt>''grid-auto-flow: dense''
<dd>
When ''grid-auto-flow: dense'' <a spec=css-grid-1>automatic placement</a>
changes the relative order of an item
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are so many different orderings referenced around here, so the "relative order" is not much clear.

When ''grid-auto-flow: dense'' <a spec=css-grid-1>automatic placement</a>
changes the relative order of an item
with respect to other items with ''reading-order/auto'' or ''reading-order/0'' 'reading-order'
by shifting it earlier in the grid placement order,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand "grid placement order", I guess it's the order in which the items are processed during the placement algorithm, but that algorithm has multiple steps and items may be iterated in multiple of them so it's not clear.

Should this instead refer to the "grid-modified document order (with the major axis as specified by grid-auto-flow)" from the example?

the UA will match the [=reading and navigation order=] of the elements
to match the layout order of their boxes as follows:

<dl>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What else will be included here? Masonry for grid seems likely. What about flex-direction: *-reverse?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only randomized orders, so masonry might (does it actually randomize?) but flex-direction: *-reverse wouldn't.

@rachelandrew
Copy link
Contributor

@fantasai I've been discussing this with @chrishtr and @bfgeek and I wanted to bring that back to this PR.

TL:DR is that we are wondering if we only need reading-order:auto (on the children) and if there are reasons to do reading-order: <integer> at all, or initially. Do compelling enough use cases exist to have that level of control or do people really just want to follow the layout?

We also wonder what will happen with row and column reversing in flexbox as people may expect different behavior there.

Longer notes below:

My original suggestion was a switch on the grid/flex container. In that scenario all of the following would follow the grid or flex modified order:

  • flex layout with order
  • flex layout with *-reverse
  • grid layout doing placement with lines/template areas
  • grid layout with order
  • "randomized grid layouts" with dense packing or masonry

In the existing edits plus this PR, the reading-order: auto value is applied to the children, and only covers:

  • "randomized grid layouts" with dense packing (and I'm assuming masonry)

To do the following you would need reading-order: <integer> on the children:

  • flex layout with order
  • flex layout with *-reverse
  • grid layout doing placement with lines/template areas
  • grid layout with order

There are definitely use cases for people wanting to follow the layout created by placement (usually because they want different layouts at different responsive breakpoints), so if we only do auto as specced, we don't have a method to achieve those use cases, and if we go for the switch on the container that likely stops us doing this version later.

An alternative, however, would be if the only value for reading-order was auto (applied to the children) and auto applied in all layout types, not just the randomized ones. As that shouldn't roadblock us from adding the integer value in the future.

Reading order and layout order being separated, is a use case. For example, this, but I think in that case, if we went with auto applied everywhere that doesn't need to affect layout-order with no reading-order.

I feel that having the property on the children is better than my idea of a switch on the parent (though I guess we could then add a switch on the parent that sets them all as a group, as with align-items), because it allows for more flexibility, but I think we could get away with that initially being auto only.

@fantasai
Copy link
Collaborator Author

fantasai commented Feb 8, 2023

@rachelandrew One of my goals here was to make it impossible for an author to apply * { use-layout-order-as-speech-order } and thereby convince themselves that a) they don't need to manage source order correctly and b) they've solved accessibility properly for the whole page. That was my primary objection to your original proposal (as noted in the original issue), and why I proposed something different that requires the author to consider what they're doing for each level of layout more explicitly.

@rachelandrew
Copy link
Contributor

One of my goals here was to make it impossible for an author to apply * { use-layout-order-as-speech-order } and thereby convince themselves that a) they don't need to manage source order correctly and b) they've solved accessibility properly for the whole page.

I get that, however it does seem odd to be making people do more work just in case they do something bad. Having reading-order: <integer> is only useful in those cases where the developer doesn't want the document order OR the grid/flex-modified order. I can't think of any good cases for that, so we are adding this additional complexity purely to make it harder for developers to do a thing they want to do. Do we do this anywhere else?

I think it would be worth bringing this issue back to the WG, my comment above and @fantasai's response explain the issue reasonably well I think.

@chrishtr
Copy link
Contributor

Agenda+ to consider @rachelandrew 's proposed simplification of the proposal. I agree with her arguments.

@Loirooriol
Copy link
Contributor

I still don't understand what's supposed to happen in grid. Grid is 2D, this is 1D, it's not straightforward.

"relative order of an item" is not well defined when there are so many different order to choose from. "grid placement order" makes no sense when grid placement has different steps that iterate different sets of items in different orders. I don't understand "grid layout doing placement with lines/template areas". "grid modified order" is not a single thing, the major axis needs to be specified.

@atanassov atanassov added this to Overflow in March 2023 VF2F Mar 7, 2023
@astearns astearns moved this from Overflow to Monday - Mar 13 in March 2023 VF2F Mar 11, 2023
@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-display-4] Define 'reading-order: auto' #7387, and agreed to the following:

  • RESOLVED: Land the PR, after addressing pending feedback.
The full IRC log of that discussion <TabAtkins> rachelandrew: reading order for grid and flex
<TabAtkins> rachelandrew: elika wrote in the spec edits for reading-order:<integer>
<TabAtkins> rachelandrew: Lets you change the order relative tot he dom
<TabAtkins> rachelandrew: and "auto" which only works for things that are "randomized" layout, like dense-order packing
<TabAtkins> rachelandrew: Where the author can't predict ahead of time what the order is
<TabAtkins> rachelandrew: As I was writing an article for dev feedback, it was very obvious that for most things we just need reading-order:auto on the children
<TabAtkins> rachelandrew: And make it apply to all layout methods
<TabAtkins> rachelandrew: If they wanted order to follow layout order, if children have "auto" this happens
<TabAtkins> rachelandrew: Elika doesn't want this bc she doesn't want to make it too easy for devs to do this (setting layout order rather than changing dom order)
<TabAtkins> rachelandrew: But it seems v obvious that we'll be making things ergonomically difficult
<TabAtkins> rachelandrew: Seems a bit odd, want to make sure we really do intend that.
<TabAtkins> fantasai: I haven't seen your examples, so less context, but
<TabAtkins> fantasai: The thing I'm concerned with is devs won't think about speec order, just set `* { reading-order:auto }` and think they've solved the problem
<TabAtkins> fantasai: But this doesn't actually use the right behaviors in many cases
<TabAtkins> fantasai: Many cases where people reorder source in order to get the speech order and layout order to be different on purpose, bc the two perceive the document differently
<TabAtkins> fantasai: For example, an image and a heading, you want the image above the heading in layout, you want th eheading first in source
<TabAtkins> fantasai: So the reason we added 'order' was to enable that case specifically, not to enable "can sort anything with the 'order' property"
<TabAtkins> rachelandrew: Those cases are handled by "order" already, as long as you don't set r-o:auto
<TabAtkins> fantasai: Yeah, my concern is just that people will set it on "*" in their stylesheet
<fremy> q+
<TabAtkins> rachelandrew: Are those people already failing to do this stuff correctly?
<TabAtkins> fantasai: I think there are people who care about a11y but not deeply, and will learn a best practice is to set it everywhere
<TabAtkins> rachelandrew: We can publish agaisnt that.
<TabAtkins> rachelandrew: My concern is that we're adding complexity just to prevent things
<TabAtkins> rachelandrew: Another concern is that if we say "auto" is only for random layouts we can't change that in the future; while we were just integers it was still open.
<TabAtkins> fantasai: I'm fine with adding a different keyword for randomized layout and using "auto" for this purpose.
<iank_> q+
<astearns> ack fremy
<TabAtkins> fremy: I think I agree it doesn't make sense for "auto" to not use layout info when you can. I think it'll be what people want most of the time.
<TabAtkins> fremy: I disagree it can be used for everything. For grid there's no obvious reading order, mentioned in the issue
<TabAtkins> fremy: "auto" makes it seem like there's always an obvious choice
<TabAtkins> fremy: We can't ask authors to add 1, 2, 3, etc; can't do that with an unknown number of elements.
<TabAtkins> fremy: I think this needs a more specific proposal for layout-by-layout.
<TabAtkins> fremy: So I don't think it makes a lot of sense for grid, don't buy that "you just need auto" is true
<TabAtkins> rachelandrew: I think more work is needed, but - what I'd like is more feedback from devs.
<TabAtkins> rachelandrew: Atm we'll be asking for feedback with a suggested spec that is making things difficult, that might affect the feedback.
<TabAtkins> rachelandrew: There were def issues around flex-direction, for example
<astearns> ack iank_
<TabAtkins> rachelandrew: But we couldn't think of use-cases where using "auto" didn't work, when discussing it with Chrishtr and others.
<TabAtkins> iank_: as-is, with integer value, it's not really adding value to webdevs
<TabAtkins> iank_: Don't think people will actively use it
<TabAtkins> astearns: next steps?
<TabAtkins> fantasai: I'd like to pull rachel's discussion out of the PR and into an issue
<TabAtkins> fantasai: The PR was about following up what was already resolved, just needed Oriol's review on technical aspects.
<TabAtkins> fantasai: So I'd like to land Oriol's feedback, and discuss additional changes in an issue.
<TabAtkins> fantasai: Havne't looked at Rachel's article so don't have the same context
<florian> q?
<TabAtkins> fantasai: But I am def concerned that there will be articles saying "just use 'auto' to handle speech"
<TabAtkins> florian: Seems fremy's direction wouldn't take us there - if we have several autoish values there's no single answer to rely on.
<TabAtkins> iank_: And in flexbox you sometimes want opposite order for reading. For grid, could want column-major and row-major.
<TabAtkins> iank_: So there are some high-level directionalities for particular layouts.
<TabAtkins> florian: Also the overlapping capabilities of grid pose possibilities.
<TabAtkins> florian: Dunno if we need to expose all of them with keywords, but shoudl think thru it
<TabAtkins> astearns: Rachel would you be okay landing this PR and opening an issue?
<TabAtkins> astearns: So resolution today is accept this PR once Oriol's concerns are handled?
<TabAtkins> fantasai: Yeah, wanna address Oriol's concerns.
<TabAtkins> florian: Want to inline an issue into the PR saying "auto" name can change?
<TabAtkins> fantasai: Yeah, can do, pointing at RAchel's issue. Might be redesigning the feature, rathe rthan just renaming
<TabAtkins> astearns: So resolution is we land this PR after addressing feedback
<TabAtkins> RESOLVED: Land the PR, after addressing pending feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
March 2023 VF2F
Monday - Mar 13
Development

Successfully merging this pull request may close these issues.

None yet

7 participants