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-anchor-1] The anchor-scroll property name is confusing/overloaded #8675

Closed
xiaochengh opened this issue Apr 3, 2023 · 7 comments · Fixed by #9040
Closed

[css-anchor-1] The anchor-scroll property name is confusing/overloaded #8675

xiaochengh opened this issue Apr 3, 2023 · 7 comments · Fixed by #9040

Comments

@xiaochengh
Copy link
Contributor

"CSS anchor-scroll" and "CSS scroll anchoring" are completely irrelevant things, but the names are almost identical, which may lead to confusions.

More notes/clarifications are needed. Or if possible, rename the property.

@tabatkins
Copy link
Member

Very much open to alternate names. Suggestions?

@wangxianzhu
Copy link

A clarification paragraph in each spec (css-scroll-anchroing and css-anchor-positioning) may be good enough.

Some SEO measures to improve google search results of "anchor-scroll" will be good, too. Currently the first Google search result of css "anchor-scroll" is about overflow-anchor from css-scroll-anchoring.

@xiaochengh
Copy link
Contributor Author

Another idea: do we really need to expose anchor-scroll to developers? Especially now it defaults to the default/implicit anchor element, and there really doesn't seem to be any legit use case of anchor-scroll: none

Maybe we can just keep it as an internal detail of calculating the snapshotted scroll offset that:

  • If the element has a default anchor element that is acceptable, use that element
  • Otherwise, if the element has an implicit anchor element that is acceptable, use that element
  • Otherwise, use some anchor element from some anchor() or anchor-size() (with some rules when multiple anchors are used)

This only makes the behavior less controllable (for a developer) when using multiple anchors in different scrollers; But again, the current anchor-scroll is already broken in this case

@tabatkins
Copy link
Member

Yeah, I suppose that works? I'd probably just do the first two bullet points, tho; trying to magically infer an element from the anchor functions seems fragile and confusing. The authoring advice will probably end up just being "use anchor-default and don't specify the anchor manually in the function, unless you're doing something special".

@xiaochengh
Copy link
Contributor Author

xiaochengh commented Jun 21, 2023

The authoring advice will probably end up just being "use anchor-default and don't specify the anchor manually in the function, unless you're doing something special".

But that means scrolling will break for sure when "doing something special", including all use cases that involve multiple anchor elements, which is still an important use case we want to support.

And it doesn't need to be too magical. For example, find the lexicographically first property with an anchor() or anchor-size(), and then use the anchor element in the first anchor function there.

@tabatkins
Copy link
Member

But that means scrolling will break for sure when "doing something special", including all use cases that involve multiple anchor elements, which is still an important use case we want to support.

That's fine, you can still set anchor-default and then just manually specify all your anchors. Or use anchor-default for your "main" anchor (which'll be recommended by tutorials, I assume) and manually specify the rest.

For example, find the lexicographically first property with an anchor() or anchor-size(), and then use the anchor element in the first anchor function there.

That's exactly the sort of unpredictability I'm talking about. Switching from setting 'top' to setting 'bottom' can change whether or not an anchor() in 'left' gets used for the scroll adjustment or not. It's deterministic, but with no author-facing rhyme or reason behind it; we just set up a completely arbitrary ordering that they can't predict without reading the spec.

For an example of a predictable behavior, we could go with "if all your anchor() functions are for the same target element, use that as the scroll adjuster". This would handle the most common case in a predictable and useful way, but it would cause scroll adjustment to break if they anchored to a second element, especially if they only did it sometimes, like in a fallback.

@xiaochengh
Copy link
Contributor Author

I see. Then let's hide anchor-scroll and just use the default/implicit anchor element.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 7, 2023
See w3c/csswg-drafts#8675 for context.

This patch:

1. Changes the implementation to longer expose `anchor-scroll` as a
   CSS property, but use the target of `anchor-default` for scrolling
   purposes

2. Coverts tests and UA style to stop using `anchor-scroll`. The usage
   is either simply removed if there's already a default anchor, or
   changed to use `anchor-default` otherwise. The cases that assume
   different `anchor-scroll` and `anchor-default` targets are removed
   since they will no longer be a possible.

To minimize diff, this patch focuses on the implementation change.
A follow up patch will convert all in-code documentations and rename
existing classes/functions to stop referring to the functionality as
"anchor-scroll", and hence, stop the naming confusion.

Bug: 1463085
Change-Id: I3cb2000de67b2b81b4edf8b9a1303de682946232
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 7, 2023
See w3c/csswg-drafts#8675 for context.

This patch:

1. Changes the implementation to longer expose `anchor-scroll` as a
   CSS property, but use the target of `anchor-default` for scrolling
   purposes

2. Coverts tests and UA style to stop using `anchor-scroll`. The usage
   is either simply removed if there's already a default anchor, or
   changed to use `anchor-default` otherwise. The cases that assume
   different `anchor-scroll` and `anchor-default` targets are removed
   since they will no longer be a possible.

To minimize diff, this patch focuses on the implementation change.
A follow up patch will convert all in-code documentations and rename
existing classes/functions to stop referring to the functionality as
"anchor-scroll", and hence, stop the naming confusion.

Bug: 1463085
Change-Id: I3cb2000de67b2b81b4edf8b9a1303de682946232
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 10, 2023
See w3c/csswg-drafts#8675 for context.

This patch:

1. Changes the implementation to longer expose `anchor-scroll` as a
   CSS property, but use the target of `anchor-default` for scrolling
   purposes

2. Coverts tests and UA style to stop using `anchor-scroll`. The usage
   is either simply removed if there's already a default anchor, or
   changed to use `anchor-default` otherwise. The cases that assume
   different `anchor-scroll` and `anchor-default` targets are removed
   since they will no longer be a possible.

To minimize diff, this patch focuses on the implementation change.
A follow up patch will convert all in-code documentations and rename
existing classes/functions to stop referring to the functionality as
"anchor-scroll", and hence, stop the naming confusion.

Bug: 1463085
Change-Id: I3cb2000de67b2b81b4edf8b9a1303de682946232
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 10, 2023
See w3c/csswg-drafts#8675 for context.

This patch:

1. Changes the implementation to longer expose `anchor-scroll` as a
   CSS property, but use the target of `anchor-default` for scrolling
   purposes

2. Coverts tests and UA style to stop using `anchor-scroll`. The usage
   is either simply removed if there's already a default anchor, or
   changed to use `anchor-default` otherwise. The cases that assume
   different `anchor-scroll` and `anchor-default` targets are removed
   since they will no longer be a possible.

To minimize diff, this patch focuses on the implementation change.
A follow up patch will convert all in-code documentations and rename
existing classes/functions to stop referring to the functionality as
"anchor-scroll", and hence, stop the naming confusion.

Bug: 1463085
Change-Id: I3cb2000de67b2b81b4edf8b9a1303de682946232
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4662290
Reviewed-by: Xianzhu Wang <[email protected]>
Commit-Queue: Xiaocheng Hu <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1168282}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 10, 2023
See w3c/csswg-drafts#8675 for context.

This patch:

1. Changes the implementation to longer expose `anchor-scroll` as a
   CSS property, but use the target of `anchor-default` for scrolling
   purposes

2. Coverts tests and UA style to stop using `anchor-scroll`. The usage
   is either simply removed if there's already a default anchor, or
   changed to use `anchor-default` otherwise. The cases that assume
   different `anchor-scroll` and `anchor-default` targets are removed
   since they will no longer be a possible.

To minimize diff, this patch focuses on the implementation change.
A follow up patch will convert all in-code documentations and rename
existing classes/functions to stop referring to the functionality as
"anchor-scroll", and hence, stop the naming confusion.

Bug: 1463085
Change-Id: I3cb2000de67b2b81b4edf8b9a1303de682946232
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4662290
Reviewed-by: Xianzhu Wang <[email protected]>
Commit-Queue: Xiaocheng Hu <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1168282}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jul 20, 2023
…nternal, a=testonly

Automatic update from web-platform-tests
[anchor-position] Make 'anchor-scroll' internal

See w3c/csswg-drafts#8675 for context.

This patch:

1. Changes the implementation to longer expose `anchor-scroll` as a
   CSS property, but use the target of `anchor-default` for scrolling
   purposes

2. Coverts tests and UA style to stop using `anchor-scroll`. The usage
   is either simply removed if there's already a default anchor, or
   changed to use `anchor-default` otherwise. The cases that assume
   different `anchor-scroll` and `anchor-default` targets are removed
   since they will no longer be a possible.

To minimize diff, this patch focuses on the implementation change.
A follow up patch will convert all in-code documentations and rename
existing classes/functions to stop referring to the functionality as
"anchor-scroll", and hence, stop the naming confusion.

Bug: 1463085
Change-Id: I3cb2000de67b2b81b4edf8b9a1303de682946232
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4662290
Reviewed-by: Xianzhu Wang <[email protected]>
Commit-Queue: Xiaocheng Hu <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1168282}

--

wpt-commits: c84d96c2e6d1062cf8690a2931a046f4a1e6cbd7
wpt-pr: 40929
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Jul 21, 2023
…nternal, a=testonly

Automatic update from web-platform-tests
[anchor-position] Make 'anchor-scroll' internal

See w3c/csswg-drafts#8675 for context.

This patch:

1. Changes the implementation to longer expose `anchor-scroll` as a
   CSS property, but use the target of `anchor-default` for scrolling
   purposes

2. Coverts tests and UA style to stop using `anchor-scroll`. The usage
   is either simply removed if there's already a default anchor, or
   changed to use `anchor-default` otherwise. The cases that assume
   different `anchor-scroll` and `anchor-default` targets are removed
   since they will no longer be a possible.

To minimize diff, this patch focuses on the implementation change.
A follow up patch will convert all in-code documentations and rename
existing classes/functions to stop referring to the functionality as
"anchor-scroll", and hence, stop the naming confusion.

Bug: 1463085
Change-Id: I3cb2000de67b2b81b4edf8b9a1303de682946232
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4662290
Reviewed-by: Xianzhu Wang <wangxianzhuchromium.org>
Commit-Queue: Xiaocheng Hu <xiaochenghchromium.org>
Cr-Commit-Position: refs/heads/main{#1168282}

--

wpt-commits: c84d96c2e6d1062cf8690a2931a046f4a1e6cbd7
wpt-pr: 40929

UltraBlame original commit: 849e1d77375fff083bc5612e5d5388f38aac1720
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Jul 21, 2023
…nternal, a=testonly

Automatic update from web-platform-tests
[anchor-position] Make 'anchor-scroll' internal

See w3c/csswg-drafts#8675 for context.

This patch:

1. Changes the implementation to longer expose `anchor-scroll` as a
   CSS property, but use the target of `anchor-default` for scrolling
   purposes

2. Coverts tests and UA style to stop using `anchor-scroll`. The usage
   is either simply removed if there's already a default anchor, or
   changed to use `anchor-default` otherwise. The cases that assume
   different `anchor-scroll` and `anchor-default` targets are removed
   since they will no longer be a possible.

To minimize diff, this patch focuses on the implementation change.
A follow up patch will convert all in-code documentations and rename
existing classes/functions to stop referring to the functionality as
"anchor-scroll", and hence, stop the naming confusion.

Bug: 1463085
Change-Id: I3cb2000de67b2b81b4edf8b9a1303de682946232
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4662290
Reviewed-by: Xianzhu Wang <wangxianzhuchromium.org>
Commit-Queue: Xiaocheng Hu <xiaochenghchromium.org>
Cr-Commit-Position: refs/heads/main{#1168282}

--

wpt-commits: c84d96c2e6d1062cf8690a2931a046f4a1e6cbd7
wpt-pr: 40929

UltraBlame original commit: 849e1d77375fff083bc5612e5d5388f38aac1720
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Jul 21, 2023
…nternal, a=testonly

Automatic update from web-platform-tests
[anchor-position] Make 'anchor-scroll' internal

See w3c/csswg-drafts#8675 for context.

This patch:

1. Changes the implementation to longer expose `anchor-scroll` as a
   CSS property, but use the target of `anchor-default` for scrolling
   purposes

2. Coverts tests and UA style to stop using `anchor-scroll`. The usage
   is either simply removed if there's already a default anchor, or
   changed to use `anchor-default` otherwise. The cases that assume
   different `anchor-scroll` and `anchor-default` targets are removed
   since they will no longer be a possible.

To minimize diff, this patch focuses on the implementation change.
A follow up patch will convert all in-code documentations and rename
existing classes/functions to stop referring to the functionality as
"anchor-scroll", and hence, stop the naming confusion.

Bug: 1463085
Change-Id: I3cb2000de67b2b81b4edf8b9a1303de682946232
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4662290
Reviewed-by: Xianzhu Wang <wangxianzhuchromium.org>
Commit-Queue: Xiaocheng Hu <xiaochenghchromium.org>
Cr-Commit-Position: refs/heads/main{#1168282}

--

wpt-commits: c84d96c2e6d1062cf8690a2931a046f4a1e6cbd7
wpt-pr: 40929

UltraBlame original commit: 849e1d77375fff083bc5612e5d5388f38aac1720
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants