Skip to content

Conversation

@KianNH
Copy link
Member

@KianNH KianNH commented Jan 16, 2025

Summary

Adopts eslint for JavaScript, TypeScript, React, Astro and accessibility.

Moves all .jsx components to .tsx.

Cleans up some redundant Tailwind classes in favour of DEFAULT cases on black and accent colours.

Adds reviewdog in CI for PR comments when eslint produces an error.

Added a third option to addTooltip to pass custom props to tippy, and a custom hideAfter implementation.

Rewrote RuleID and OneTrust to resolve accessibility issues with non-interactive elements used for interactivity.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jan 16, 2025

Deploying cloudflare-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5c121e0
Status: ✅  Deploy successful!
Preview URL: https://53f9a3af.cloudflare-docs-7ou.pages.dev
Branch Preview URL: https://kian-pcx-15396.cloudflare-docs-7ou.pages.dev

View logs

contentLines = contentLines.filter(
(line) => !/<[\/]?docs-tag name=".*">/.test(line),
(line) => !/<[/]?docs-tag name=".*">/.test(line),
Copy link
Member Author

Choose a reason for hiding this comment

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

No need to escape [/] when not using PCRE (which is only PHP really).

@@ -1,4 +1,5 @@
---
/* eslint-disable astro/jsx-a11y/no-noninteractive-tabindex */
Copy link
Member Author

Choose a reason for hiding this comment

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

Improving the accessibility of this component isn't feasible at the moment, we will tackle it when we migrate from Tippy.js to floating-ui.

return "Function calling";
}

return [];
Copy link
Member Author

Choose a reason for hiding this comment

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

Prevents (... | undefined)[] type due to a hole-y array.

@@ -1,44 +0,0 @@
import { useState } from "react";
Copy link
Member Author

Choose a reason for hiding this comment

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

This is not used and was a WIP that we do not plan on finishing at the moment.

isCurrent: boolean;
badge: ComponentProps<typeof Badge> | undefined;
attrs: LinkHTMLAttributes;
attrs: HTMLAttributes<"a">;
Copy link
Member Author

Choose a reason for hiding this comment

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

Whilst the linkHTMLAttributesSchema code was taken from Starlight's codebase, we are happy with HTMLAttributes.

if (indexPage.data.sidebar.group?.hideIndex) {
group.entries.splice(indexIdx, 1)[0];
group.entries.splice(indexIdx, 1);
Copy link
Member Author

Choose a reason for hiding this comment

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

[0] was an unused expression - no functionality change.

Comment on lines +242 to +244
& > * a[aria-current="page"] {
color: var(--sl-color-accent-high) !important;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Uses the same orange as the logo when highlighting the current page on dark mode.


// omit sort_date from output
const { sort_date, ...data } = x.data;
const { sort_date: _, ...data } = x.data;
Copy link
Member Author

Choose a reason for hiding this comment

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

_ to indicate it is unused, we just do this to omit sort_date from data.

@KianNH KianNH requested a review from kodster28 as a code owner January 17, 2025 14:14
@KianNH KianNH requested a review from dcpena as a code owner January 17, 2025 17:17
@github-actions
Copy link
Contributor

@@ -0,0 +1 @@
save-exact=true
Copy link
Member Author

Choose a reason for hiding this comment

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

This is to prevent scenarios where npm install and npm ci can result in different versions, due to the ^... selectors that are typical.

With exact selectors ("foo": "0.1.0" vs "foo": "^0.1.0"), regardless of npm i or npm ci, a package at 0.1.0 should always install 0.1.0.

Comment on lines +82 to +83
"react": "19.0.0",
"react-dom": "19.0.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

We updated to React 19 but it results in no visual or functionality changes for us.

Comment on lines +3 to +4
import tsBlankSpace from "ts-blank-space";
import { format } from "prettier";
Copy link
Member Author

Choose a reason for hiding this comment

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

detype is slower and the latest versions cause build errors due to using a CJS shim.

Copy link
Collaborator

@kodster28 kodster28 left a comment

Choose a reason for hiding this comment

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

Did some spot testing and looks good to me.

Post release, should confirm OT functionality.

@KianNH KianNH merged commit a1bf485 into production Jan 21, 2025
12 checks passed
@KianNH KianNH deleted the kian/PCX-15396 branch January 21, 2025 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants