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

Fold LinkStyle (a mixin) into its inheriting interfaces #5669

Merged
merged 6 commits into from
Jun 9, 2021
Merged

Fold LinkStyle (a mixin) into its inheriting interfaces #5669

merged 6 commits into from
Jun 9, 2021

Conversation

queengooborg
Copy link
Collaborator

This PR removes the pages for the LinkStyle mixin and remaps the references of its only member (sheet) to the individual interfaces. There is no page already in place for the member, unfortunately, so the link will remain a 404 for the time being.

Note: this is my first PR for the MDN content (though not my first content edit altogether), and especially my first attempt at a change like this. Please let me know if I missed anything!

Corresponding BCD PR: mdn/browser-compat-data#10724

@queengooborg queengooborg requested review from a team as code owners June 4, 2021 09:28
@queengooborg queengooborg requested review from Rumyra and removed request for a team June 4, 2021 09:28
Copy link
Contributor

@teoli2003 teoli2003 left a comment

Choose a reason for hiding this comment

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

There is a mention of LinkStyle in https://developer.mozilla.org/en-US/docs/Web/API/SVGStyleElement . I think we should add the sheet method here too (like in HTMLStyleElement, HTMLLinkElement, ProcessingInstruction).

I've not double checked, but I think doing rg LinkStyle * in api/ to check that every mentions of LinkStyle have been dealt with may be useful. (rg is a recursive version of grep).

Very happy to see LinkStyle going away.

@queengooborg queengooborg requested a review from a team June 4, 2021 10:17
@github-actions
Copy link
Contributor

github-actions bot commented Jun 4, 2021

Preview URLs

Flaws

URL: /en-US/docs/Web/SVG/SVG_2_support_in_Mozilla
Title: SVG 2 support in Mozilla
on GitHub
Flaw count: 19

  • macros:
    • /en-US/docs/Web/API/SVGColor does not exist
    • /en-US/docs/Web/API/SVGICCColor does not exist
    • /en-US/docs/Web/API/SVGElementInstance does not exist
    • /en-US/docs/Web/API/SVGElementInstanceList does not exist
    • /en-US/docs/Web/CSS/vector-effect does not exist
    • and 14 more flaws omitted

URL: /en-US/docs/Web/API/SVGStyleElement
Title: SVGStyleElement
on GitHub
Flaw count: 3

  • macros:
    • /en-US/docs/Web/API/SVGStyleElement/type does not exist
    • /en-US/docs/Web/API/SVGStyleElement/media does not exist
    • /en-US/docs/Web/API/SVGStyleElement/title does not exist

URL: /en-US/docs/Web/API/CSSStyleSheet
Title: CSSStyleSheet
on GitHub
Flaw count: 4

  • macros:
    • /en-US/docs/Web/API/HTMLLinkElement/sheet does not exist
    • /en-US/docs/Web/API/HTMLStyleElement/sheet does not exist
    • /en-US/docs/Web/API/SVGStyleElement/sheet does not exist
    • /en-US/docs/Web/API/ProcessingInstruction/sheet does not exist

URL: /en-US/docs/Web/API/CSS_Object_Model
Title: CSS Object Model (CSSOM)
on GitHub
Flaw count: 7

  • macros:
    • /en-US/docs/Web/API/CSSCharsetRule does not exist
    • /en-US/docs/Web/API/CSSFontFeatureValuesMap does not exist
    • /en-US/docs/Web/API/CSSFontFeatureValuesRule does not exist
    • /en-US/docs/Web/API/CSSMarginRule does not exist
    • /en-US/docs/Web/API/CSSVariablesMap does not exist
    • and 2 more flaws omitted

URL: /en-US/docs/Web/API/HTMLStyleElement
Title: HTMLStyleElement
on GitHub
Flaw count: 7

  • macros:
    • /en-US/docs/Web/API/HTMLStyleElement/disabled does not exist
    • wrong xref macro used (consider changing which macro you use)
    • /en-US/docs/Web/API/HTMLStyleElement/sheet does not exist
    • wrong xref macro used (consider changing which macro you use)
  • broken_links:
    • Can't resolve /en-US/docs/DOM/Using_dynamic_styling_information
  • bad_bcd_links:
    • no explanation!
    • no explanation!

URL: /en-US/docs/Web/API/HTMLLinkElement
Title: HTMLLinkElement
on GitHub
Flaw count: 14

  • macros:
    • /en-US/docs/Web/API/HTMLLinkElement/crossOrigin does not exist
    • /en-US/docs/Web/API/HTMLLinkElement/disabled does not exist
    • /en-US/docs/Web/API/HTMLLinkElement/href does not exist
    • /en-US/docs/Web/API/HTMLLinkElement/hreflang does not exist
    • /en-US/docs/Web/API/HTMLLinkElement/media does not exist
    • and 7 more flaws omitted
  • bad_bcd_links:
    • no explanation!
    • no explanation!

URL: /en-US/docs/Mozilla/Firefox/Releases/46
Title: Firefox 46 for developers
on GitHub
Flaw count: 7

  • macros:
    • /en-US/docs/Web/CSS/@font/font-display does not exist
    • /en-US/docs/Web/CSS/@font does not exist
    • /en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakSet/clear does not exist
    • /en-US/docs/Web/JavaScript/Reference/Operators/Array_comprehensions does not exist
    • /en-US/docs/Web/JavaScript/Reference/Operators/Generator_comprehensions does not exist
    • and 2 more flaws omitted

External URLs

URL: /en-US/docs/Web/SVG/SVG_2_support_in_Mozilla
Title: SVG 2 support in Mozilla
on GitHub

No new external URLs


URL: /en-US/docs/Web/API/SVGStyleElement
Title: SVGStyleElement
on GitHub

No new external URLs


URL: /en-US/docs/Web/API/CSSStyleSheet
Title: CSSStyleSheet
on GitHub

No new external URLs


URL: /en-US/docs/Web/API/CSS_Object_Model
Title: CSS Object Model (CSSOM)
on GitHub


URL: /en-US/docs/Web/API/HTMLStyleElement
Title: HTMLStyleElement
on GitHub

No new external URLs


URL: /en-US/docs/Web/API/HTMLLinkElement
Title: HTMLLinkElement
on GitHub

No new external URLs


URL: /en-US/docs/Mozilla/Firefox/Releases/46
Title: Firefox 46 for developers
on GitHub

No new external URLs

(this comment was updated 2021-06-08 13:22:38.053554)

Copy link
Collaborator

@Rumyra Rumyra left a comment

Choose a reason for hiding this comment

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

Hi @vinyldarkscratch 👋 this looks awesome 😎

Seems tests are failing on redirect because the sheet property page doesn't exist. Basically it seems it just never got written... I think possibly for the moment we just redirect any Link/sheet urls to Web/API/HTMLLinkElement

I've added a suggestion, but happy to have a better one :)

files/en-us/_redirects.txt Show resolved Hide resolved
@@ -115,7 +115,7 @@ <h4 id="WebRTC">WebRTC</h4>
<h4 id="New_APIs">New APIs</h4>

<ul>
<li>In SVG, the {{domxref("SVGStyleElement")}} interface now implements the {{domxref("LinkStyle")}} mixin ){{bug(1239128)}}.</li>
<li>In SVG, the {{domxref("SVGStyleElement")}} interface now implements the {{domxref("SVGStyleElement.sheet")}} member ){{bug(1239128)}}.</li>
Copy link
Contributor

Choose a reason for hiding this comment

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

In release notes, let's not change any wording, but just remove the domxref and use just <code> instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree!

@@ -98,7 +98,7 @@ <h2 id="Specifications">Specifications</h2>
<tr>
<td>{{SpecName('DOM2 HTML', 'html.html#ID-35143001', 'HTMLLinkElement')}}</td>
<td>{{Spec2('DOM2 HTML')}}</td>
<td>Added a second inheritence, the {{domxref("LinkStyle")}} interface.</td>
<td>Added a second inheritence, the LinkStyle mixin, exposing the {{domxref("HTMLLinkElement.sheet")}} member.</td>
Copy link
Contributor

Choose a reason for hiding this comment

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

In spec tables I would also leave the text unchanged, just replace the domxref.

Copy link
Contributor

@foolip foolip left a comment

Choose a reason for hiding this comment

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

Looks good apart from one small nit.

files/en-us/web/api/cssstylesheet/index.html Outdated Show resolved Hide resolved
@foolip
Copy link
Contributor

foolip commented Jun 8, 2021

@Rumyra might you be able to review this? If you agree with my suggestion, just applying it an merging should be OK.

Co-authored-by: Philip Jägenstedt <philip@foolip.org>
@Rumyra
Copy link
Collaborator

Rumyra commented Jun 8, 2021

Approved @foolip - I'm unsure of the status of the two unresolved conversations above - are we ok to merge? Happy to do so :)

@foolip
Copy link
Contributor

foolip commented Jun 8, 2021

@Rumyra those issues have been resolved. I'm not sure why there's no "resolve" button, but they're outdated at any rate.

@Rumyra Rumyra merged commit 0931f33 into mdn:main Jun 9, 2021
@Rumyra
Copy link
Collaborator

Rumyra commented Jun 9, 2021

@foolip weirdly I do have a resolve button hence the query 🤷🏻‍♀️ anyways thank you 🙏 all merged ☺️

@queengooborg queengooborg deleted the demix-LinkStyle branch June 9, 2021 20:47
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants