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

[cssom-view] Clarify definition of scrolling box #1540

Closed
wants to merge 1 commit into from

Conversation

zcorpan
Copy link
Member

@zcorpan zcorpan commented Jun 19, 2017

Fixes #1526.

@dbaron
Copy link
Member

dbaron commented Aug 22, 2017

Is there a way to review a generated version that shows what these link to? It's not clear to me what spec that would be in...

@zcorpan
Copy link
Member Author

zcorpan commented Aug 23, 2017

Generated HTML is

   <p>An element or <a data-link-type="dfn" href="#viewport" id="ref-for-viewport">viewport</a> has an associated <dfn class="dfn-paneled" data-dfn-type="dfn" data-noexport="" id="scrolling-box">scrolling box</dfn> if it has a <a data-link-type="dfn" href="https://drafts.csswg.org/css-overflow-3/#scroll-container" id="ref-for-scroll-container">scroll
container</a> and <a data-link-type="dfn" href="https://drafts.csswg.org/css-overflow-3/#scrollable-overflow" id="ref-for-scrollable-overflow">scrollable overflow</a>. This box is the same as the <a data-link-type="dfn" href="https://drafts.csswg.org/css-overflow-3/#scrollport" id="ref-for-scrollport">scrollport</a>. <a data-link-type="biblio" href="#biblio-css-overflow-3">[CSS-OVERFLOW-3]</a></p>

@tigercosmos
Copy link

Shall we land this PR?

@dbaron
Copy link
Member

dbaron commented May 4, 2018

@zcorpan - Sorry for taking so long to get back to this:

It's not clear to me why you're conditioning this on having scrollable overflow. (And, actually, I don't think it's really defined what having scrollable overflow means. I'd generally interpret that to mean that something that is included in the scrollable overflow (as the term is defined) actually overflows the padding-box. Strictly speaking, it's defined as a set, so it's always there.) If your intent is to condition it on whether scrollable overflow is overflowing the padding box... I'm not sure why you'd want to do that. My instinct would be to define the concept for everything that is a scroll container. It seems cleaner, and not doing that seems likely to be problematic for something like overflow: scroll without any overflow.

I'm also a little worried about element/box terminology and has/is terminology. A scroll container is defined as a thing that boxes are. This seems to say it's a thing that elements have. I think it's probably good to use boxes. I don't actually care about has vs. is except for being consistent.

@dbaron
Copy link
Member

dbaron commented May 4, 2018

Also, if it's the same as the scrollport, why make a new term rather than using that term?

Copy link
Member

@dbaron dbaron left a comment

Choose a reason for hiding this comment

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

see 2 previous comments

@dbaron
Copy link
Member

dbaron commented May 4, 2018

Also, I guess I've looked through the uses of the term now (which I probably should have done earlier), and I suppose I see that given the uses are all related to the APIs that cause movement on scrolling, why you'd want to condition it on having scrollable overflow. However, it seems like that usage precludes things like being able to control whether removal of content (if it causes a box to no longer have overflow) triggers a smooth or instant scroll. So even given that usage, it seems better to have the term be consistently defined whether or not there's currently overflow, and handle all cases of the scrollable range changing consistently, whether or not the change in the scrollable range means there's no longer any room for movement. And it seems like the restrictions on the range you're able to scroll to should be able to handle all the cases that not being able to scroll at all can.

(I'd also note that sometimes there's scrollable overflow that doesn't cause any ability to scroll, e.g., if it's above the scrollport.)

@zcorpan
Copy link
Member Author

zcorpan commented May 6, 2018

Replacing with scrollport seems good. I don't recall why I defined it like this.

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

Successfully merging this pull request may close these issues.

None yet

3 participants