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

Add tabbed mode spec #95

Merged
merged 42 commits into from
Apr 29, 2024
Merged

Add tabbed mode spec #95

merged 42 commits into from
Apr 29, 2024

Conversation

mgiuca
Copy link
Member

@mgiuca mgiuca commented Mar 6, 2024

Based on an earlier draft by @loubrett (Louise Brett).


Preview | Diff

@mgiuca mgiuca requested a review from loubrett March 6, 2024 06:04
@mgiuca mgiuca marked this pull request as draft March 6, 2024 06:04
@mgiuca
Copy link
Member Author

mgiuca commented Mar 7, 2024

@loubrett Ready for you to look now.

@mgiuca mgiuca marked this pull request as ready for review March 7, 2024 06:21
Copy link
Collaborator

@loubrett loubrett left a comment

Choose a reason for hiding this comment

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

Just some minor comments about linking stuff.

I haven't thought through the URLPattern stuff any more yet so I'll have a think about that next week.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@mgiuca
Copy link
Member Author

mgiuca commented Mar 15, 2024

I just pushed a major rework. Still incomplete so don't re-review yet.

@mgiuca mgiuca marked this pull request as draft March 15, 2024 07:09
mgiuca added 22 commits March 27, 2024 16:17
Copy-pasted from Louise Brett's source doc. Need to clean up, linkify and edit.
Now there is just a single processing section at the h2 level.
Too numerous to list individual changes. A lot of moving around.

Many additional details added, including:
- List of invariants that must be followed (e.g. home tab never
  navigates out of home scope, regular tab never navigates into home
  scope).
- Specifying that home tabs open at the start_url.
- Properly defining within and outside of home scope terms.
- Properly defining what happens when you click the new tab button.
mgiuca added 6 commits April 19, 2024 11:52
Define a boolean condition first, rather than defining a set. Makes it more readable.

Also exclude fragments (but not query).
…non-normatively, and just talk about it as an idea, not a formal term.

Add a note about ignoring the fragment but not the query.
Do not use the at-risk string syntax.

Add more detail about the default matching the query, or not.
Now, the three invariants are non-normative discussion of what is intended, rather than a hard rule. The normative logic elsewhere should speak for itself.

This way, we do not need to formally find a concept of 'initial URL' and can just informally talk about documents that rewrite their URL.
@mgiuca mgiuca marked this pull request as ready for review April 22, 2024 07:28
@mgiuca
Copy link
Member Author

mgiuca commented Apr 22, 2024

Hi @loubrett . This is finally ready for review. PTAL; note that you can preview it here.

@mgiuca mgiuca requested a review from dmurph April 22, 2024 07:31
@mgiuca
Copy link
Member Author

mgiuca commented Apr 22, 2024

@dmurph adding as editor. PTAL, thanks, and let me know if you want to discuss!

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
Copy link
Collaborator

@dmurph dmurph left a comment

Choose a reason for hiding this comment

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

Some initial comments - also happy to go over in-person in a meeting

index.html Show resolved Hide resolved
index.html Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Show resolved Hide resolved
Now has an explicit concept of "has a new tab button" (analogous to "has
a home tab"). The parser algorithm no longer (accidentally) sets the new
tab URL to the start URL if it is within home tab scope.

Add a note clarifying why the new tab button is not allowed to be in
home tab scope.

Also note the start URL default, which is easy to miss as it is buried
in the parser algorithm.
mgiuca added 4 commits April 24, 2024 16:28
Done in consultation with dmurph and loubrett, to make the navigation rules much clearer and hook into the HTML navigable machinery.
@mgiuca
Copy link
Member Author

mgiuca commented Apr 26, 2024

@dmurph @loubrett Finished responding to both of your reviews. Thanks for the insightful comments & meeting this morning.

See my comments above - I was not able to do literally what we discussed but I think I have captured the behaviour you wanted. PTAL.

Copy link
Collaborator

@dmurph dmurph left a comment

Choose a reason for hiding this comment

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

LGTM, although nit: the display_mode link in the rendered text seems broken.

Copy link
Collaborator

@loubrett loubrett left a comment

Choose a reason for hiding this comment

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

LGTM

@mgiuca
Copy link
Member Author

mgiuca commented Apr 29, 2024

LGTM, although nit: the display_mode link in the rendered text seems broken.

Thanks Dan.

Yes, I mentioned this to you in a chat: we need to wait until w3c/csswg-drafts#7306 gets pushed out to the editor's draft, and then it will automatically resolve itself (because the definition of "display mode" was moved from CSS mediaqueries-5 to Manifest; Manifest updated right away, CSS did not). I don't know when that will be; it was last updated in May 2022, coming up to two years...

Edit: filed an issue about this.

@mgiuca mgiuca merged commit 6f48374 into WICG:gh-pages Apr 29, 2024
1 check passed
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.

3 participants