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

fix(macros/cssxref): correct URL generation for data types and functions #8766

Merged
merged 6 commits into from
Mar 28, 2024

Conversation

yarusome
Copy link
Contributor

@yarusome yarusome commented May 4, 2023

Summary

Fixes #8755.

Problem

  1. {{CSSXref}} Generated URLs for CSS functions contain extra parentheses.
  2. :host() and fit-content() are not handled correctly.
  3. <overflow> is not handled correctly.
  4. The examples in cssxref.ejs are outdated.

Solution

  1. Remove parentheses in slug.
  2. Append _funtion to slug.
  3. Add the data type into the special-case block.
  4. Update examples according to current results.

How did you test this change?

Run the following code in the console:

// Only return the URL
function CSSXref(arg0, arg1 = "", arg2 = "") {
  const lang = "en-US"; // for convenience
  let url = "";
  let urlWithoutAnchor = "";
  let displayName = (arg1 || arg0);
  
  // Deal with CSS data types and functions by removing <> and ()
  let slug = arg0.replace(/&lt;(.*)&gt;/g, "$1")
      .replace(/\(\)/g, "");
  
  // Special case <color>, <flex>, <overflow>, and <position>
  if (/^&lt;(color|flex|overflow|position)&gt;$/.test(arg0)) {
      slug += "_value";
  }
  
  // Special case :host() and fit-content()
  if (/^(:host|fit-content)\(\)$/.test(arg0)) {
      slug += "_function";
  }
  
  const basePath = `/${lang}/docs/Web/CSS/`;
  urlWithoutAnchor = basePath + slug;
  url = urlWithoutAnchor + arg2;
  
  return url;
}

// Test cases adpated from 'cssxref.test.ts'
const TEST_CASE = [
  "display",
  "attr()",
  ":host()",
  "length",
  "&lt;length&gt;",
  "&lt;color&gt;",
  "&lt;overflow&gt;"
];

for(const caseInput of TEST_CASE) {
  console.log(CSSXref(caseInput));
}

/* Expected output:
   /en-US/docs/Web/CSS/display
   /en-US/docs/Web/CSS/attr
   /en-US/docs/Web/CSS/:host_function
   /en-US/docs/Web/CSS/length
   /en-US/docs/Web/CSS/length
   /en-US/docs/Web/CSS/color_value
   /en-US/docs/Web/CSS/overflow_value
*/

@github-actions github-actions bot added the macros tracking issues related to kumascript macros label May 4, 2023
@yarusome
Copy link
Contributor Author

yarusome commented May 4, 2023

The unit test is complaining about pageType (?)

@yarusome yarusome marked this pull request as ready for review May 4, 2023 13:16
@caugner
Copy link
Contributor

caugner commented May 4, 2023

@yarusome The unit test does not access mdn/content, but instead uses some mock pages, and the mocked attr page still uses parentheses:

attr: {
url: [CSS_BASE_URL, "attr()"].join("/"),
data: {
url: [CSS_BASE_URL, "attr()"].join("/"),
summary:
'The <strong><code>attr()</code></strong> <a href="/en-US/docs/Web/CSS">CSS</a> function is used to retrieve the value of an attribute of the selected element and use it in the style sheet.',
pageType: "css-function",
},
},

Updating those urls should fix the failing test.

@yarusome
Copy link
Contributor Author

yarusome commented May 5, 2023

@caugner Thanks for the hint. All unit tests pass now.

@caugner caugner requested a review from a team as a code owner November 8, 2023 16:59
@github-actions github-actions bot added the idle label Jan 24, 2024
@github-actions github-actions bot removed the idle label Mar 2, 2024
@caugner caugner changed the title fix(macros): correct URL generation for CSS data types and functions fix(macros/cssxref): correct URL generation for data types and functions Mar 21, 2024
Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

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

@caugner caugner merged commit a0813d0 into mdn:main Mar 28, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
macros tracking issues related to kumascript macros
Projects
None yet
Development

Successfully merging this pull request may close these issues.

{{CSSXref}} handles CSS functions incorrectly
2 participants