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-overflow-4] Define scroll-markers #10243

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

Conversation

flackr
Copy link
Contributor

@flackr flackr commented Apr 23, 2024

Defines scroll-markers properties, pseudo-elements, scrolltarget attribute and associated behaviors for #9745.

@flackr
Copy link
Contributor Author

flackr commented Apr 23, 2024

I'm sure there's a lot of important details missing here, but it might be useful to have something written down to point to and work out the details over PRs and issues. WDYT?

@flackr flackr requested a review from fantasai April 23, 2024 18:12
@tabatkins
Copy link
Member

(ping @dbaron @fantasai @frivoal )

css-overflow-4/Overview.bs Outdated Show resolved Hide resolved
css-overflow-4/Overview.bs Outdated Show resolved Hide resolved
css-overflow-4/Overview.bs Outdated Show resolved Hide resolved
css-overflow-4/Overview.bs Outdated Show resolved Hide resolved
css-overflow-4/Overview.bs Outdated Show resolved Hide resolved
css-overflow-4/Overview.bs Outdated Show resolved Hide resolved
css-overflow-4/Overview.bs Outdated Show resolved Hide resolved
css-overflow-4/Overview.bs Outdated Show resolved Hide resolved
css-overflow-4/Overview.bs Outdated Show resolved Hide resolved
Copy link
Contributor Author

@flackr flackr left a comment

Choose a reason for hiding this comment

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

Thanks for the review @tabatkins!

css-overflow-4/Overview.bs Outdated Show resolved Hide resolved
css-overflow-4/Overview.bs Outdated Show resolved Hide resolved
css-overflow-4/Overview.bs Outdated Show resolved Hide resolved
css-overflow-4/Overview.bs Outdated Show resolved Hide resolved
css-overflow-4/Overview.bs Outdated Show resolved Hide resolved
flackr and others added 2 commits May 10, 2024 13:16
Co-authored-by: Tab Atkins Jr. <jackalmage@gmail.com>
@flackr flackr requested a review from tabatkins May 10, 2024 17:18
@fantasai
Copy link
Collaborator

I'm sure there's a lot of important details missing here, but it might be useful to have something written down to point to and work out the details over PRs and issues. WDYT?

I agree, but the plan was to put it into Overflow L5 along with the current fragmentation appendix, not to patch Overflow L4. :) #9745 (comment)

Wrt the content of this, I think it needs more explanation of the use cases it's trying to solve (and the use case it's not trying to solve). For my part, I don't understand why we aren't trying to solve the "highlight the relevant table of contents entry" item along with this, for example.

The part about only the currently-active scroll marker being focusable is also weird. How do you activate a scroll marker control with the keyboard if it's not focusable?

aarongable pushed a commit to chromium/chromium that referenced this pull request May 27, 2024
::scroll-marker-group pseudo element acts as an area to which
::scroll-marker pseudo elements are flowed. It is a container for
`navigation dots` of carousel (e.g. dots for choosing image in gallery).

::scroll-marker-group is the first/last child of the originating element,
depending on the scroll-marker property (after/before/none).
Note: if there is ::view-transition pseudo on the originating element,
it would be the last child instead.

Note: this CL places box of ::scroll-marker-group as a child of
originating element instead of prev/next sibling of it for now.
This will be fixed in following CLs.

No AX actions are needed, since ::scroll-marker-group acts as a container
for ::scroll-marker pseudo elements.
AX tests will be added with ::scroll-marker CL.

Spec PR: w3c/csswg-drafts#10243

Bug: 332396355
Change-Id: I269c92f9dfe2d13c9595991ee371355f0046bf8f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5545381
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Daniil Sakhapov <sakhapov@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1306382}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants