sideshowbarker/support multiple bcd queries caugner rebased #6358
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
|
https://github.com/mdn/content/tree/sideshowbarker/api-spec-urls-browser-compat-array (mdn/content#16267) has all the existing docs with multiple BCD tables (multiple It’s |
|
This pull request has merge conflicts that must be resolved before it can be merged. |
Without this change, when there are multiple BCD features, the Specifications section won’t get populated as expected. I tested this change and I believe it’ll work correctly for all cases. (And I think we can switch to Object.values rather than Object.entries, because we only need the values, and don’t really need to do anything with the keys.)
This adds some checks for “undefined” (which will happen when a browser-compat array value contains any bad BCD feature ID).
…ueries Some features — e.g., css.properties.justify-content — have no compat data themselves but have subfeatures with compat data. In the case where we’re processing a browser-compat value which is an array — that is, multiple top-level features in one table (in contrast to one feature and its subfeatures) — unless we explicitly check for the __compat-less-feature-with-subfeatures case, and handle it, we can end up with rows for expected features being entirely missing from the rendered BCD table. So this change adds handling for that case.
…CD queries Some features — e.g., css.properties.justify-content — have no compat data themselves but have subfeatures with compat data. In the case where we’re processing a browser-compat value which is an array — that is, multiple top-level features in one table (in contrast to one feature and its subfeatures) — unless we explicitly check for the __compat-less-feature-with-subfeatures case, and handle it, we can end up with expected specifications being entirely missing from the rendered Specifications section. So this change adds handling for that case.
When we’re showing a row for a subfeature in a BCD table, the subfeature name or description on its own may not provide enough context — especially in the case where we have a BCD table with multiple top-level features (that is, from a browser-compat value which is an array) — so for that case, we show the (parent) feature name — acquired from the current mdn_url value — as a prefix before the subfeature name/description.
In _getBCDData(queryString), the queryString value) can be, e.g., "html.elements.link.rel,html.elements.a.rel,html.elements.area.rel" for the case where we’re processing multiple BCD queries. So as the key in the data structure we create there and pass around to other parts of the code, to ensure the key is unique, we use the full BCD query/feature name rather than some portion of it. Otherwise, if we use, e.g., just the part after the last dot, then for the example above, that ends up being just 'rel' for all three 'query' values from that full queryString value. And because 'rel' isn’t unique, if we used it as the key in our internal data structure, then in each iteration through the 'queryString', we’d end up setting a new value for the 'rel' key — and clobbering whatever value the previous iteration set. And then — to make things very clear — we pass around that key/property to other parts of the code using the variable 'query' to identify it (rather than 'name'). And then later, for the case when we’re processing multiple BCD 'query' instances (BCD queries/features) in the part of the code that outputs the rendered BCD table — where a shortened identifier is needed — we can end up with duplicate identifiers being displayed unless we further qualify them. The query string "html.elements.label.for,html.elements.output.for", for example, would result in a rendered BCD table that has just 'for' as the feature name in each row. So in this case, we take the last _two_ parts of the query/feature name — that is, e.g., 'label.for' and 'output.for' — and display those in the rendered table.
This change adds an HTML “title” attribute too all BCD table row headers, so that users can hover to see the full BCD feature name. Otherwise, without this change, for some multiple-BCD-queries cases, the displayed names of the features are the same; for example, consider "css.properties.width.fit-content_function,css.properties.grid-template-columns.fit-content" For the case of those two features, the displayed feature name for both is just “fit-content()” — so there’s no way for users to distinguish which row applies to which feature. But with this change, users can hover the table row headers to see which is for which.
|
Closing in favor of #6464. |
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.
Summary
This extends #6004 to add handling for more cases discovered during testing.
Problem description (from #6004)
Previously, pages could only reference a single BCD query.
Solution (from #6004)
document-extractor.jsto use pure functions for BCD and Specifications.BCD_BROWSERSandBCD_BROWSER_RELEASESconstants.Additional changes in this PR branch