Miscellaneous improvements for the WPT element #2010
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Never mind this comment, I've changed my mind.I previously proposed here some tentative changes to how things could be styled (see below), but have since found a better way and included that in the pull request.Also, as a possible alternative to the right floats for the closed state, another variant I quite like is with inline-blocks. You can try it out by applying this after the latest commit. diff --git a/bikeshed/wpt/wptElement.py b/bikeshed/wpt/wptElement.py
index 9c6b614a9..76295f62b 100644
--- a/bikeshed/wpt/wptElement.py
+++ b/bikeshed/wpt/wptElement.py
@@ -323,18 +323,14 @@ wptStyle = """
color: var(--wpt-border);
}
.wpt-tests-block:not([open]){
- border-left: none;
- background: none;
margin: 0;
- padding: 0;
- border-top: 1px dotted var(--wpt-border);
+ padding-bottom: 0;
+ border: none;
opacity: 0.5;
+ display: inline-block;
}
.wpt-tests-block:not([open]) summary{
- background: var(--wpt-bg);
padding: 0;
- float: right;
- max-width: max-content;
font-size: 0.8em;
line-height: 1;
} |
Lists can be pretty long and get in the way of spec readability otherwise.
The pathprefix attribute of the wpt element has done its job by the time we generate the output, and it is a non-standard one, so remove it
Document the fact that the hidden attribute on wpt elements lets you exclude them from the output, while letting bikeshed check their contents. And while we're at it, remove them entirely from the output. Relates to #1895
Also, take advantage of that to add a description of wpt blocks in the "Document conventions" sections of boilerplate files
Having multiple successive wpt elements in the source can happen due to wanting a different pathprefix, but there's no reason to keep this separation in the output since the prefixes have been processed away.
Most readers of a spec are not here for the tests, so wpt elements, particularly in their closed state, should get out of the way, and disturb the general typography of the document as little as possible. Hot pink wasn't the ideal color for that (and it's very close to the w3c-chosen purple color used for candidate corrections anyway), so this commit replaces that with gray. In the closed state, wpt elements are made small, semi-transparent, right aligned, and fitting into the margin between the elements they annotate.
@fantasai had a look at the output and liked it, so I've tentatively included this in the PR as well. |
|
All right, finally had a chance to look at this, and it looks great. I'll probably actually log a breaking change to make "closed" the default, so people can prep by turning it off if they want. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Here's a loosely related set of tweaks, fixes, and new features to how the wpt elements work. I suppose I could send that as a bunch of separate PRs, but that seems like unnecessary overhead.
Some issues covered by this:
Not sure if this is easier to review commit by commit or not, but each commit should be clean, so you can if you want to, and I suspect the commit messages will give a bit of context about why I'm doing what I'm doing. One of the commits (4e53842) changes the indentation of a large block of code, so at least that one is probably easier to review with
--ignore-space-changeSome of these choices are probably subjective, but I'm pretty happy with the result. Let me know if anything isn't to your liking.
I think that with these in place, I'd be happy to turn
WPT Displayon by default on all my EDs: it would encourage readers in general (and implementers in particular) to look up the results of tests as they read various sections of the spec to see how well implementations are doing (and possibly file/fix bugs). Also, it would make it easier to notice when some part of the document doesn't have tests, and encourage writing them. (As for turning it on on TR, we'd probably have to sync up with pubrules and tr-design, but one thing at a time).Maybe we could add
"WPT Display": "closed"toboilerplate/csswg/defaults-ED.include. Besides giving us a good bunch of annotated EDs, that'll nicely populate the tests as well.