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-scroll-snap-1] Unclear what mapping the 4-value longhands use #1050

Closed
plinss opened this Issue Feb 21, 2017 · 8 comments

Comments

Projects
None yet
4 participants
@plinss
Member

plinss commented Feb 21, 2017

scroll-padding and scroll-snap-margin shorthands are unclear as to which longhand properties are set (specifically physical or logical).

My guess is that the intent is the shorthand follows the standard TRBL pattern, and then physical sides are mapped to logical longhands based on writing mode, but this is not clear from reading the spec.

@tabatkins tabatkins changed the title from [css-scroll-snap-1] to [css-scroll-snap-1] Unclear what mapping the 4-value longhands use May 15, 2017

@tabatkins

This comment has been minimized.

Show comment
Hide comment
@tabatkins

tabatkins May 15, 2017

Member

We specify this:

The <length> values give outsets (interpreted as for margin or border-image-outset).

Member

tabatkins commented May 15, 2017

We specify this:

The <length> values give outsets (interpreted as for margin or border-image-outset).

@plinss

This comment has been minimized.

Show comment
Hide comment
@plinss

plinss May 20, 2017

Member

Sorry, this doesn't address my concern at all, first the link to 'border-image-outset' is completely irrelevant as it has neither physical or logical longhands, nor any way to set the values in logical directions (afaict). Second, the link to 'margin' in css2.1 has no reference to or mention of logical properties.

The concern here came up during TAG review, scroll-snap specifies a shorthand, and both physical and logical longhands, but has absolutely no information that explains how the shorthand maps to the longhands, and the links given don't help. While I could make an educated guess, others couldn't. At the least, a brief sentence explaining that the shorthand sets all the longhands, in the manner described by css-logical §3.2 (or a better link) would be helpful. I get that this is probably obvious to the editors, but it's not obvious to a casual reader (which is the real issue).

Member

plinss commented May 20, 2017

Sorry, this doesn't address my concern at all, first the link to 'border-image-outset' is completely irrelevant as it has neither physical or logical longhands, nor any way to set the values in logical directions (afaict). Second, the link to 'margin' in css2.1 has no reference to or mention of logical properties.

The concern here came up during TAG review, scroll-snap specifies a shorthand, and both physical and logical longhands, but has absolutely no information that explains how the shorthand maps to the longhands, and the links given don't help. While I could make an educated guess, others couldn't. At the least, a brief sentence explaining that the shorthand sets all the longhands, in the manner described by css-logical §3.2 (or a better link) would be helpful. I get that this is probably obvious to the editors, but it's not obvious to a casual reader (which is the real issue).

@fantasai

This comment has been minimized.

Show comment
Hide comment
@fantasai

fantasai Jun 19, 2017

Contributor

OK, I checked in some changes. @plinss could you review?

Contributor

fantasai commented Jun 19, 2017

OK, I checked in some changes. @plinss could you review?

@plinss

This comment has been minimized.

Show comment
Hide comment
@plinss

plinss Jun 19, 2017

Member

It's an improvement (and I accept that you gave me exactly what I asked for), but I still think it could be better.

  1. the description of the scroll-snap-margin and scroll-snap-padding shorthands are referring to margin and padding respectively in CSS2.x, but neither margin nor padding in CSS2.x have any information about how the shorthands relate to the logical longhand properties
  2. I accept that css-logical does define how the margin and padding shorthands relate to the physical and logical properties, but since css-logical doesn't list the scroll-snap properties, then css-scroll-snap should define that interaction. (I do see the first paragraph of appendix A, but it leaves a bit to be inferred.)
  3. Appendix A goes on to describe the scroll-padding-* and scroll-snap-margin-* longhands as being longhands for scroll-padding and scroll-snap-margin, but has no description of the physical/logical split.
  4. There's no mention of the 'logical' keyword being valid in the values of scroll-padding or scroll-snap-margin (unless I missed it).

Reading css-scroll-snap without having css-logical fully swapped in, can lead a reader to think that a declaration of scroll-padding: 1em will set both the physical and logical longhands, where my (current) understanding of css-logical is that it should only set the physical longhands and leave the logical longhands alone. (Where scroll-padding: logical 1em would set only the logical longhands.)

Perhaps a paragraph in each of the physical and flow-realtive longhand descriptions in Appendix A stating that the physical longhands are only set by the shorthand when the 'logical' is not present and the flow-relative longhands are only set when the 'logical' keyword is present...

Member

plinss commented Jun 19, 2017

It's an improvement (and I accept that you gave me exactly what I asked for), but I still think it could be better.

  1. the description of the scroll-snap-margin and scroll-snap-padding shorthands are referring to margin and padding respectively in CSS2.x, but neither margin nor padding in CSS2.x have any information about how the shorthands relate to the logical longhand properties
  2. I accept that css-logical does define how the margin and padding shorthands relate to the physical and logical properties, but since css-logical doesn't list the scroll-snap properties, then css-scroll-snap should define that interaction. (I do see the first paragraph of appendix A, but it leaves a bit to be inferred.)
  3. Appendix A goes on to describe the scroll-padding-* and scroll-snap-margin-* longhands as being longhands for scroll-padding and scroll-snap-margin, but has no description of the physical/logical split.
  4. There's no mention of the 'logical' keyword being valid in the values of scroll-padding or scroll-snap-margin (unless I missed it).

Reading css-scroll-snap without having css-logical fully swapped in, can lead a reader to think that a declaration of scroll-padding: 1em will set both the physical and logical longhands, where my (current) understanding of css-logical is that it should only set the physical longhands and leave the logical longhands alone. (Where scroll-padding: logical 1em would set only the logical longhands.)

Perhaps a paragraph in each of the physical and flow-realtive longhand descriptions in Appendix A stating that the physical longhands are only set by the shorthand when the 'logical' is not present and the flow-relative longhands are only set when the 'logical' keyword is present...

@plinss plinss reopened this Jun 19, 2017

@fantasai

This comment has been minimized.

Show comment
Hide comment
@fantasai

fantasai Jun 20, 2017

Contributor

The logical keyword is not currently valid in scroll-padding or scroll-snap-margin, because that part of css-logical is insufficiently stable (note the bright red issue!). Whether you treat the shorthand as setting all longhands or only the physical ones is actually irrelevant: both interpretations yield the same result. I'd be perfectly happy to say the shorthands only set the physical longhands, but then we may need to update the spec later if we decide otherwise for css-logical.

Contributor

fantasai commented Jun 20, 2017

The logical keyword is not currently valid in scroll-padding or scroll-snap-margin, because that part of css-logical is insufficiently stable (note the bright red issue!). Whether you treat the shorthand as setting all longhands or only the physical ones is actually irrelevant: both interpretations yield the same result. I'd be perfectly happy to say the shorthands only set the physical longhands, but then we may need to update the spec later if we decide otherwise for css-logical.

@plinss

This comment has been minimized.

Show comment
Hide comment
@plinss

plinss Jun 20, 2017

Member

Ok, so if css-logical isn't stable enough then I accept that it doesn't make sense to be overly specific in this spec. Perhaps an additional sentence in the beginning of Appendix A indicating that the mapping between the shorthand and longhand properties is also determined by css-logical? With that I'd be satisfied.

(I'm also not sure that the shorthand setting all longhands vs just the physical/logical longhands is irrelevant, while the end result may not be observable in layout/scroll behavior, it will be observable in the CSSOM.)

Member

plinss commented Jun 20, 2017

Ok, so if css-logical isn't stable enough then I accept that it doesn't make sense to be overly specific in this spec. Perhaps an additional sentence in the beginning of Appendix A indicating that the mapping between the shorthand and longhand properties is also determined by css-logical? With that I'd be satisfied.

(I'm also not sure that the shorthand setting all longhands vs just the physical/logical longhands is irrelevant, while the end result may not be observable in layout/scroll behavior, it will be observable in the CSSOM.)

@fantasai

This comment has been minimized.

Show comment
Hide comment
@fantasai

fantasai Jun 21, 2017

Contributor

Ok, added 393d993
Let me know if that looks okay!

Contributor

fantasai commented Jun 21, 2017

Ok, added 393d993
Let me know if that looks okay!

triple-underscore added a commit to triple-underscore/triple-underscore.github.io that referenced this issue Jun 24, 2017

[css-scroll-snap] 各種更新
Commits on Jun 17, 2017

Convert period to semicolon per lawyers demands.
w3c/csswg-drafts#1305
w3c/csswg-drafts@0a7ab91
e81cf1be236

Commits on Jun 20, 2017

Update Changes list.
w3c/csswg-drafts@617e94f
ec7c6599f3b

Rearrange prose to be clearer about value mapping to longhands, link to
css-logical.
Fixes w3c/csswg-drafts#1050 .
w3c/csswg-drafts@38caa2e
debcac0cc21

Commits on Jun 22, 2017
add that shorthand interactions are also governed by css-logical
w3c/csswg-drafts@393d993
6fff215f060
@plinss

This comment has been minimized.

Show comment
Hide comment
@plinss

plinss Jun 27, 2017

Member

Works for me, thanks!

Member

plinss commented Jun 27, 2017

Works for me, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment