[WEB-3426] Modernize docs to latest Ably-UI Standards #2251
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
aralovelace
force-pushed
the
web-3426-css-changes-to-modernize-docs
branch
from
6fc72bd to
406c9ee
Compare
aralovelace
changed the title
aralovelace
force-pushed
the
web-3426-css-changes-to-modernize-docs
branch
from
72498ea to
adca1a8
Compare
jamiehenson
force-pushed
the
web-3426-css-changes-to-modernize-docs
branch
from
adca1a8 to
89d9894
Compare
jamiehenson
force-pushed
the
web-3426-css-changes-to-modernize-docs
branch
from
aefd249 to
d3b142f
Compare
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request introduces various changes across multiple components and configuration files, primarily focusing on updating the Node.js version, modifying CSS class names, and reorganizing imports. Notably, the CircleCI configuration is updated to use Node.js 20.13.1, and several components have adjusted their styling classes to align with a new design system. Additionally, some files have been deleted, including configurations for social media links and certain TypeScript declaration files. Overall, the changes aim to modernize the codebase and improve styling consistency. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant CircleCI
participant NodeJS
User->>App: Trigger build
App->>CircleCI: Start build process
CircleCI->>NodeJS: Use Node.js 20.13.1
NodeJS->>CircleCI: Build completed
CircleCI->>App: Notify build status
App->>User: Display build results
Assessment against linked issues
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
jamiehenson
force-pushed
the
web-3426-css-changes-to-modernize-docs
branch
from
c4f79ea to
819bdc6
Compare
jamiehenson
force-pushed
the
web-3426-css-changes-to-modernize-docs
branch
from
819bdc6 to
bd03f99
Compare
jamiehenson
force-pushed
the
web-3426-css-changes-to-modernize-docs
branch
from
bd03f99 to
cc7370b
Compare
jamiehenson
force-pushed
the
web-3426-css-changes-to-modernize-docs
branch
from
a359810 to
2ce0d3d
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (9)
src/components/Footer/FooterTopNav/FooterSocial/FooterSocialMenu.tsx (2)
5-5: Consider keeping props for better component reusability.While removing props simplifies the component, it creates tight coupling with the data file. Consider keeping the props interface with a default value from the imported data:
-export const FooterSocialMenu = () => ( +export const FooterSocialMenu = ({ links = ablySocialLinks }: { links?: Array<{ label: string; link: string }> }) => (This approach would:
- Maintain backward compatibility
- Make the component more reusable
- Facilitate easier testing
- Allow for runtime data updates if needed
Line range hint
6-14: Implementation looks good with a minor type improvement suggestion.The component implementation is clean and follows React best practices. Consider extracting the social link type for better type safety:
+type SocialLink = { label: string; link: string }; + export const FooterSocialMenu = () => ( <> <p className="mt-24 ui-text-p3">Find us on:</p> <div className="pt-16 flex items-center"> - {ablySocialLinks.map((social: { label: string; link: string }) => ( + {ablySocialLinks.map((social: SocialLink) => ( <FooterSocialMenuIcons label={social.label} link={social.link} key={social.label} /> ))} </div> </> );src/components/blocks/software/Pre.tsx (3)
Line range hint
47-106: Consider extracting SDK interface logic into a custom hookThe SDK interface handling logic is complex and could benefit from being extracted into a custom hook for better maintainability and reusability. This would also make it easier to add proper error handling and loading states.
Example implementation:
// useSDKInterface.ts export const useSDKInterface = (altData: Record<string, any>, initialInterface = DEFAULT_PREFERRED_INTERFACE) => { const [selectedSDKInterface, setSelectedSDKInterface] = useState(initialInterface); const [previousSDKInterface, setPreviousSDKInterface] = useState(''); useEffect(() => { // Move the SDK interface selection logic here if (altData) { const allRealtimeData = Object.entries(altData) .map(([language]) => language?.includes(`${REALTIME_SDK_INTERFACE}_`) ? language.split('_', 2)[1] : '', ) .filter(Boolean); // Add your existing logic here } }, [altData, selectedSDKInterface]); return { selectedSDKInterface, setSelectedSDKInterface, previousSDKInterface, setPreviousSDKInterface, }; };
Line range hint
189-195: Add TypeScript return types and JSDoc documentationThe utility functions would benefit from explicit return type annotations and JSDoc documentation for better maintainability and IDE support.
Example implementation:
/** * Removes SDK interface prefix from language string if present * @param language - The language string that may contain SDK interface prefix * @returns The cleaned language string without SDK interface prefix */ export const cleanIfLanguageHasSDKInterface = (language: string): string => language.includes(`${REALTIME_SDK_INTERFACE}_`) || language.includes(`${REST_SDK_INTERFACE}_`) ? language.split('_', 2)[1] : language; /** * Filters and transforms language list based on selected SDK interface * @param allLanguage - Array of all available languages * @param selectedSDKInterface - Currently selected SDK interface * @returns Array of languages that match the selected SDK interface */ const getLanguagesSDKInterface = (allLanguage: string[], selectedSDKInterface: string): string[] => allLanguage.map((language) => (language.includes(`${selectedSDKInterface}_`) ? language : '')).filter((s) => s);
Line range hint
156-169: Enhance accessibility for info messageThe info message section could benefit from improved accessibility support for screen readers.
Example implementation:
- <aside className="mb-16 flex justify-between items-start"> + <aside className="mb-16 flex justify-between items-start" role="alert" aria-label="Language support information"> - <div className="mt-2"> + <div className="mt-2" aria-hidden="true"> <Icon name="icon-gui-info" size="1rem" /> </div> <div className="ml-8 leading-normal"> You're currently viewing the <span className="font-semibold">{languageLabel ?? pageLanguage}</span>{' '} docs. There either isn't a {languageLabel ?? pageLanguage} code sample for this example, or this feature isn't supported in {languageLabel ?? pageLanguage}. Switch language to view this example in a different language, or{' '} - <a className="ui-link" href="/docs/sdks"> + <a className="ui-link" href="/docs/sdks" aria-label="Check SDK feature support documentation"> check which SDKs support this feature. </a>src/styles/global.css (1)
Legal disclaimer needs to be updated or removed
The codebase no longer uses Optimo webfonts - only JetBrains Mono font declarations are present, with Manrope referenced as a fallback font family. The current Optimo fonts legal disclaimer is irrelevant and should be removed or replaced with appropriate licensing information for JetBrains Mono.
- Remove or update the Optimo webfonts legal disclaimer in
src/styles/global.css- If needed, add appropriate licensing information for JetBrains Mono font usage
🔗 Analysis chain
Line range hint
93-108: Verify font licensing complianceThe legal disclaimer mentions restrictions for Optimo webfonts. Please verify that the new Manrope font usage complies with licensing requirements.
The previous result showed that
global.csscontains Optimo-related text, but we need to verify:
- If there are actual Optimo font declarations still in use
- How the Manrope font is declared and if it potentially conflicts with the Optimo disclaimer
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any remaining references to Optimo fonts echo "Checking for Optimo font references:" rg -l "optimo" src/Length of output: 124
Script:
#!/bin/bash # Let's check the content of global.css to understand the font declarations echo "Checking font declarations in global.css:" rg -A 5 "@font-face" src/styles/global.css # Also check for any Manrope font references echo -e "\nChecking Manrope font references:" rg "Manrope" src/styles/global.cssLength of output: 1321
tailwind.config.js (2)
34-40: Consider documenting safelist patternsThe safelist configuration is well-structured, combining both static classes and pattern matches. However, consider adding comments to explain the purpose of each pattern, especially for future maintainability.
safelist: [ ...ablyUIConfig.safelist, + // API Reference specific background colors ...Object.keys(apiReferenceSpecificColors).map((c) => `bg-${c}`), + // Highlight and notification background colors ...Object.keys(highlightColors).map((c) => `bg-${c}`), + // Standard utility classes used dynamically ...safelistStandard, + // Dynamic class patterns (docs-* and col-span-*) ...safelistGreedy, ],
Line range hint
9-24: Remove deprecated statusColors objectThe comment indicates that
statusColorsshould be removed when ably-ui 14 lands. Since this PR updates to ably-ui 15.1.0, this object should be removed.-// Remove this when ably-ui 14 lands, along with the Status component -const statusColors = { - 'status-operational': '#00e80b', - 'status-minor': '#f8c100', - 'status-major': '#ff723f', - 'status-critical': '#e40000', - 'status-unknown': '#c6ced9', -};package.json (1)
130-130: Document prettier version constraintThe addition of
prettier-2as an alias forprettier@^2should be documented to explain why this specific version is required.Add a comment in the README or documentation explaining the prettier version constraint and its purpose.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (16)
src/components/blocks/Html/__snapshots__/Html.test.tsx.snapis excluded by!**/*.snapsrc/components/blocks/software/__snapshots__/Pre.test.tsx.snapis excluded by!**/*.snapsrc/components/blocks/wrappers/__snapshots__/ConditionalChildrenLanguageDisplay.test.js.snapis excluded by!**/*.snapsrc/components/blocks/wrappers/__snapshots__/LocalLanguageAlternatives.test.tsx.snapis excluded by!**/*.snapsrc/external-scripts/__snapshots__/utils.test.ts.snapis excluded by!**/*.snapsrc/fonts/museosans/300/webfont.woffis excluded by!**/*.woffsrc/fonts/museosans/300/webfont.woff2is excluded by!**/*.woff2src/fonts/museosans/500/webfont.woffis excluded by!**/*.woffsrc/fonts/museosans/500/webfont.woff2is excluded by!**/*.woff2src/fonts/museosans/500italic/webfont.woffis excluded by!**/*.woffsrc/fonts/museosans/500italic/webfont.woff2is excluded by!**/*.woff2src/fonts/museosans/700/webfont.woffis excluded by!**/*.woffsrc/fonts/museosans/700/webfont.woff2is excluded by!**/*.woff2src/fonts/museosans/900/webfont.woffis excluded by!**/*.woffsrc/fonts/museosans/900/webfont.woff2is excluded by!**/*.woff2yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (93)
data/yaml/page-furniture/ablySocial.yaml(0 hunks)how-tos/pub-sub/README.md(1 hunks)how-tos/pub-sub/how-to.mdx(3 hunks)jest.config.js(2 hunks)package.json(5 hunks)src/components/Article/InArticleBanner/InArticleOptions.tsx(1 hunks)src/components/ButtonWithTooltip/ButtonWithTooltip.tsx(1 hunks)src/components/Footer/Footer.tsx(1 hunks)src/components/Footer/FooterTopNav.tsx(1 hunks)src/components/Footer/FooterTopNav/FooterMenu/FooterMenu.tsx(1 hunks)src/components/Footer/FooterTopNav/FooterSocial/FooterSocialMenu.tsx(1 hunks)src/components/Footer/data.ts(1 hunks)src/components/Header/Dropdown/Button/DropdownButton.tsx(2 hunks)src/components/Header/Dropdown/Contents/ContentLink.tsx(1 hunks)src/components/Header/Dropdown/Contents/Title.tsx(1 hunks)src/components/Header/Dropdown/DropdownMenu.tsx(0 hunks)src/components/Header/Dropdown/Summary/Summary.tsx(1 hunks)src/components/Header/Dropdown/Summary/SummaryDescription.tsx(1 hunks)src/components/Header/Dropdown/Summary/SummaryTitle.tsx(1 hunks)src/components/Header/HamburgerMenu/HamburgerDropdown/HamburgerDropdownFooter.tsx(1 hunks)src/components/Header/HamburgerMenu/HamburgerDropdown/HamburgerSidebarRenderer/HamburgerSidebarDropdownPopulatedItem/HamburgerSidebarDropdownButton.tsx(1 hunks)src/components/Header/HamburgerMenu/HamburgerDropdown/HamburgerSidebarRenderer/HamburgerSidebarDropdownPopulatedItem/HamburgerSidebarDropdownContents.tsx(1 hunks)src/components/Header/HamburgerMenu/HamburgerDropdown/HamburgerSidebarRenderer/HamburgerSidebarDropdownPopulatedItem/HamburgerSidebarDropdownSummary.tsx(1 hunks)src/components/Header/HamburgerMenu/HamburgerDropdown/HamburgerSidebarRenderer/HamburgerSidebarDropdownPopulatedItem/MaybeShowHamburgerSummaryLink.tsx(1 hunks)src/components/Header/Header.tsx(1 hunks)src/components/Header/SearchBar/EmptyState.tsx(1 hunks)src/components/Header/SearchBar/KeyIcon.tsx(1 hunks)src/components/Header/SearchBar/SearchBar.tsx(1 hunks)src/components/Header/SearchBar/SuggestionBox.tsx(2 hunks)src/components/Header/TopMainNavIllustration/TopMainNavAblyLogo.tsx(1 hunks)src/components/Header/TopMainNavUser/DesktopSignedOut.tsx(1 hunks)src/components/Header/TopMainNavUser/SignedIn/SignIn.tsx(1 hunks)src/components/Homepage/BodySection/BodySection.tsx(2 hunks)src/components/Homepage/BodySection/BodySectionDescription.tsx(1 hunks)src/components/Homepage/BodySection/CallToAction.tsx(0 hunks)src/components/Homepage/BodySection/Card/FeatureCard.tsx(1 hunks)src/components/Homepage/BodySection/Card/HeroCard.tsx(1 hunks)src/components/Homepage/BodySection/Card/SdkCard.tsx(1 hunks)src/components/HorizontalMenu/HorizontalMenu.tsx(1 hunks)src/components/LanguageButton/LanguageButton.test.tsx(4 hunks)src/components/LanguageButton/LanguageButton.tsx(2 hunks)src/components/Layout/Layout.tsx(2 hunks)src/components/Markdown/MarkdownProvider.tsx(2 hunks)src/components/Menu/Label/index.tsx(1 hunks)src/components/Menu/MenuItem/MenuItem.tsx(1 hunks)src/components/Menu/MenuItemButton/MenuItemButton.module.css(1 hunks)src/components/Menu/MenuItemButton/MenuItemButton.tsx(1 hunks)src/components/Notification/Notification.tsx(1 hunks)src/components/PageTitle.tsx(1 hunks)src/components/ProductNavigation/ProductNavigation.tsx(2 hunks)src/components/ProductPage/BodySection/BodySection.tsx(2 hunks)src/components/ProductPage/BodySection/BodySectionDescription.tsx(1 hunks)src/components/ProductPage/BodySection/CallToAction.tsx(0 hunks)src/components/ProductPage/BodySection/Card/ExampleCard.tsx(1 hunks)src/components/ProductPage/BodySection/Card/FeatureCard.tsx(1 hunks)src/components/ProductPage/BodySection/Card/Links.tsx(1 hunks)src/components/ProductPage/BodySection/Card/QuickstartCard.tsx(1 hunks)src/components/ProductPage/BodySection/Card/TutorialCard.tsx(1 hunks)src/components/SDKInterfacePanel/SDKInterfacePanel.tsx(1 hunks)src/components/SDKsPage/Card/index.tsx(2 hunks)src/components/SDKsPage/MainSection/index.tsx(2 hunks)src/components/SDKsPage/index.tsx(1 hunks)src/components/Select/styles.ts(0 hunks)src/components/Sidebar/ExpandableIndicator.tsx(1 hunks)src/components/Sidebar/RightSidebar/RightSidebar.tsx(1 hunks)src/components/Sidebar/RightSidebar/RightSidebarMobile.tsx(1 hunks)src/components/Sidebar/RightSidebar/RightSidebarWrapper.tsx(1 hunks)src/components/Sidebar/SectionTitle.tsx(1 hunks)src/components/Sidebar/Sidebar.tsx(2 hunks)src/components/Sidebar/SidebarHeading.tsx(3 hunks)src/components/Sidebar/SidebarItem/ExpandableAccordionHeading.tsx(2 hunks)src/components/blocks/complex-functionality/Button.tsx(1 hunks)src/components/blocks/dividers/Aside.tsx(2 hunks)src/components/blocks/external-references/A.test.tsx(1 hunks)src/components/blocks/external-references/A.tsx(1 hunks)src/components/blocks/external-references/styles.css(0 hunks)src/components/blocks/headings/H4.tsx(1 hunks)src/components/blocks/software/Code/InlineCodeElement.tsx(1 hunks)src/components/blocks/software/Code/MultilineCodeContent.tsx(2 hunks)src/components/blocks/software/Pre.tsx(2 hunks)src/components/blocks/table/Table.tsx(1 hunks)src/components/blocks/table/Th.tsx(1 hunks)src/components/blocks/typography/P/index.tsx(1 hunks)src/components/blocks/wrappers/CopyLink.tsx(2 hunks)src/pages/api/control-api.tsx(2 hunks)src/styles/fonts.js(1 hunks)src/styles/global.css(2 hunks)src/types/ably-ui-core-images/ably-ui-icon-loading.d.ts(0 hunks)src/types/ably-ui-core-images/ably-ui-logo-loading.d.ts(0 hunks)src/types/ably-ui-core-links/index.d.ts(0 hunks)src/types/ably-ui-core-scripts/index.d.ts(0 hunks)tailwind.config.js(1 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (10)
- data/yaml/page-furniture/ablySocial.yaml
- src/components/blocks/external-references/styles.css
- src/types/ably-ui-core-images/ably-ui-logo-loading.d.ts
- src/components/Homepage/BodySection/CallToAction.tsx
- src/types/ably-ui-core-images/ably-ui-icon-loading.d.ts
- src/components/Select/styles.ts
- src/types/ably-ui-core-links/index.d.ts
- src/components/ProductPage/BodySection/CallToAction.tsx
- src/components/Header/Dropdown/DropdownMenu.tsx
- src/types/ably-ui-core-scripts/index.d.ts
✅ Files skipped from review due to trivial changes (2)
- how-tos/pub-sub/README.md
- src/components/Header/SearchBar/SearchBar.tsx
🚧 Files skipped from review as they are similar to previous changes (73)
- src/components/Header/TopMainNavUser/DesktopSignedOut.tsx
- src/components/Header/TopMainNavIllustration/TopMainNavAblyLogo.tsx
- src/components/Header/Dropdown/Summary/SummaryDescription.tsx
- src/components/Header/Dropdown/Button/DropdownButton.tsx
- src/components/Footer/data.ts
- src/components/HorizontalMenu/HorizontalMenu.tsx
- src/components/Header/HamburgerMenu/HamburgerDropdown/HamburgerSidebarRenderer/HamburgerSidebarDropdownPopulatedItem/HamburgerSidebarDropdownSummary.tsx
- src/components/Header/Dropdown/Contents/Title.tsx
- src/components/Footer/FooterTopNav/FooterMenu/FooterMenu.tsx
- src/components/Menu/MenuItemButton/MenuItemButton.module.css
- src/components/ProductPage/BodySection/Card/TutorialCard.tsx
- src/components/Article/InArticleBanner/InArticleOptions.tsx
- src/components/Menu/MenuItemButton/MenuItemButton.tsx
- src/components/LanguageButton/LanguageButton.tsx
- src/components/Homepage/BodySection/Card/SdkCard.tsx
- src/components/Menu/Label/index.tsx
- src/components/blocks/software/Code/InlineCodeElement.tsx
- src/components/Sidebar/SidebarItem/ExpandableAccordionHeading.tsx
- src/components/Header/Dropdown/Summary/SummaryTitle.tsx
- src/components/Sidebar/ExpandableIndicator.tsx
- src/components/Sidebar/SectionTitle.tsx
- tsconfig.json
- src/components/PageTitle.tsx
- src/components/SDKsPage/MainSection/index.tsx
- src/components/Header/Dropdown/Summary/Summary.tsx
- src/components/Header/Dropdown/Contents/ContentLink.tsx
- src/components/Header/HamburgerMenu/HamburgerDropdown/HamburgerSidebarRenderer/HamburgerSidebarDropdownPopulatedItem/HamburgerSidebarDropdownContents.tsx
- src/components/Homepage/BodySection/Card/HeroCard.tsx
- src/components/Header/TopMainNavUser/SignedIn/SignIn.tsx
- src/components/Header/HamburgerMenu/HamburgerDropdown/HamburgerSidebarRenderer/HamburgerSidebarDropdownPopulatedItem/HamburgerSidebarDropdownButton.tsx
- src/components/blocks/headings/H4.tsx
- src/components/Sidebar/RightSidebar/RightSidebarMobile.tsx
- src/components/blocks/external-references/A.test.tsx
- src/components/blocks/external-references/A.tsx
- src/components/Header/HamburgerMenu/HamburgerDropdown/HamburgerDropdownFooter.tsx
- src/components/Menu/MenuItem/MenuItem.tsx
- src/components/ProductPage/BodySection/Card/QuickstartCard.tsx
- src/components/blocks/dividers/Aside.tsx
- src/components/ProductPage/BodySection/BodySectionDescription.tsx
- src/components/Homepage/BodySection/BodySectionDescription.tsx
- src/components/Header/Header.tsx
- src/components/blocks/typography/P/index.tsx
- src/components/Header/SearchBar/SuggestionBox.tsx
- src/components/blocks/table/Th.tsx
- src/components/Header/SearchBar/KeyIcon.tsx
- src/components/Footer/Footer.tsx
- src/components/Header/HamburgerMenu/HamburgerDropdown/HamburgerSidebarRenderer/HamburgerSidebarDropdownPopulatedItem/MaybeShowHamburgerSummaryLink.tsx
- src/components/Markdown/MarkdownProvider.tsx
- src/components/LanguageButton/LanguageButton.test.tsx
- src/components/blocks/wrappers/CopyLink.tsx
- src/components/ProductPage/BodySection/Card/Links.tsx
- src/components/SDKsPage/index.tsx
- src/components/Footer/FooterTopNav.tsx
- src/pages/api/control-api.tsx
- src/components/ProductNavigation/ProductNavigation.tsx
- src/components/SDKInterfacePanel/SDKInterfacePanel.tsx
- src/components/ProductPage/BodySection/Card/FeatureCard.tsx
- src/components/Sidebar/Sidebar.tsx
- src/components/Sidebar/RightSidebar/RightSidebarWrapper.tsx
- src/components/Layout/Layout.tsx
- src/components/Sidebar/RightSidebar/RightSidebar.tsx
- src/components/ButtonWithTooltip/ButtonWithTooltip.tsx
- src/components/ProductPage/BodySection/BodySection.tsx
- src/components/blocks/table/Table.tsx
- src/components/Sidebar/SidebarHeading.tsx
- src/components/ProductPage/BodySection/Card/ExampleCard.tsx
- src/components/SDKsPage/Card/index.tsx
- src/components/Notification/Notification.tsx
- jest.config.js
- src/components/Header/SearchBar/EmptyState.tsx
- src/components/blocks/complex-functionality/Button.tsx
- src/components/Homepage/BodySection/BodySection.tsx
- src/components/blocks/software/Code/MultilineCodeContent.tsx
🧰 Additional context used
📓 Learnings (1)
how-tos/pub-sub/how-to.mdx (1)
Learnt from: jamiehenson
PR: ably/docs#2251
File: how-tos/pub-sub/how-to.mdx:51-51
Timestamp: 2024-12-03T12:52:14.160Z
Learning: In the 'ably/docs' repository, unless a PR specifically mentions MDX refactoring, MDX files (`*.mdx` files) should not be reviewed.
🪛 Biome (1.9.4)
src/styles/global.css
[error] 50-50: Duplicate font names are redundant and unnecessary: Segoe
Remove duplicate font names within the property
(lint/suspicious/noDuplicateFontNames)
🪛 LanguageTool
how-tos/pub-sub/how-to.mdx
[grammar] ~74-~74: The verb form ‘explain’ does not appear to fit in this context.
Context: ...ton> ## Further reading The following reading explain the concepts of pub/sub and channels in...
(SINGULAR_NOUN_VERB_AGREEMENT)
🔇 Additional comments (10)
how-tos/pub-sub/how-to.mdx (1)
Line range hint 1-78: Skipping review of MDX file
Based on previous feedback from @jamiehenson, we should not review MDX files unless a PR specifically mentions MDX refactoring. Since this PR focuses on updating the ably-ui library and modernizing docs to latest Ably-UI standards without explicitly mentioning MDX refactoring, we'll skip reviewing this file.
🧰 Tools
🪛 LanguageTool
[grammar] ~74-~74: The verb form ‘explain’ does not appear to fit in this context.
Context: ...ton> ## Further reading The following reading explain the concepts of pub/sub and channels in...
(SINGULAR_NOUN_VERB_AGREEMENT)
src/components/Homepage/BodySection/Card/FeatureCard.tsx (1)
13-14: LGTM! Consistent with UI modernization
The typography classes (ui-text-h3, ui-text-p1) align with the new design system standards.
src/components/Footer/FooterTopNav/FooterSocial/FooterSocialMenu.tsx (1)
3-3: Verify the data file structure and consider using absolute imports.
While moving social links to a separate data file is a good practice, consider using absolute imports for better maintainability.
✅ Verification successful
Import path and data structure are correctly implemented
The verification confirms that:
- The data file exists at the correct location:
src/components/Footer/data.ts - The
ablySocialLinksexport is properly structured with social media links - The relative import path
../../datacorrectly resolves to the data file - This is the only component importing
ablySocialLinks, suggesting proper encapsulation within the Footer component
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the data file structure and exports
# Check if the data file exists and contains the expected exports
rg -A 5 "export.*ablySocialLinks" "src/components/Footer/data.ts"
# Check for any other components using this data to ensure consistency
rg "import.*ablySocialLinks" --type typescript
Length of output: 605
Script:
#!/bin/bash
# Let's try with a more reliable approach to find TypeScript files
rg "import.*ablySocialLinks" -g "*.ts" -g "*.tsx"
# Also verify the relative path resolution is correct by checking the directory structure
fd "data.ts" src/components/Footer/
Length of output: 232
src/components/blocks/software/Pre.tsx (1)
2-2: LGTM: Class name updates align with Ably-UI standards
The changes to import the cn utility and update class names from 'docs-link' to 'ui-link' are consistent with the PR's objective to modernize the UI standards.
Also applies to: 165-165
src/styles/fonts.js (1)
Line range hint 1-120: Verify font usage across components
Let's ensure all components are properly updated to handle the new font system.
✅ Verification successful
The previous search pattern might have been too restrictive. Let's broaden our search to catch different import syntaxes and direct usage of font-related properties. This will help us verify if there are any components that need to be updated with the new font system.
The previous results show that fonts are primarily used through CSS modules and global CSS, with the fonts.js being imported in index.js. Let's verify how these styles are being composed and if there are any styled-components usages that might be affected by the font changes.
Based on the collected evidence from all the searches, I can now provide a final response:
Font system changes appear to be safely contained within the styling system
The codebase follows a well-structured approach where:
- Font definitions are centrally managed in
src/styles/fonts.js - Styles are primarily applied through CSS modules (
.module.cssfiles) - The font system is exported through
src/styles/index.jsas a named export - Components use direct CSS properties rather than the font system's JavaScript exports
The changes to fonts.js don't require updates across components because:
- No direct usage of the
fontsimport was found in component files - Font styles are applied through CSS modules with hardcoded values
- Global font settings are handled through global.css
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for components using font definitions
echo "Components using font definitions:"
rg -l "import.*fonts'" src/
Length of output: 111
Script:
#!/bin/bash
# Let's try different search patterns to find font usage
echo "Searching for font imports and usage:"
rg -l "fonts['\"]" src/
echo -e "\nSearching for specific font styles:"
rg "font-size|font-weight|font-family" src/
echo -e "\nSearching for font weight constants:"
rg "weights\." src/
Length of output: 2591
Script:
#!/bin/bash
# Let's check how fonts are being imported and used in the codebase
echo "Checking fonts import in index.js:"
cat src/styles/index.js
echo -e "\nChecking for styled-components or CSS-in-JS usage:"
rg "styled|css`|css\(" src/
echo -e "\nChecking global CSS variables for fonts:"
rg "@font-face|:root|--.*font" src/styles/global.css
Length of output: 542
src/styles/global.css (3)
13-13: LGTM: Syntax highlighter integration
The addition of syntax-highlighter.css aligns with the modernization effort and improves code presentation consistency.
37-52: 🛠️ Refactor suggestion
Remove duplicate Segoe UI font entries
The font stack contains duplicate Segoe UI entries which should be consolidated.
font-family:
Manrope,
ui-sans-serif,
system-ui,
-apple-system,
BlinkMacSystemFont,
Roboto,
Helvetica Neue,
Arial,
Noto Sans,
sans-serif,
Apple Color Emoji,
- Segoe UI,
- Segoe UI Emoji,
+ Segoe UI, Segoe UI Emoji,
Segoe UI Symbol,
Noto Color Emoji;🧰 Tools
🪛 Biome (1.9.4)
[error] 50-50: Duplicate font names are redundant and unnecessary: Segoe
Remove duplicate font names within the property
(lint/suspicious/noDuplicateFontNames)
54-54: Verify impact of font size reduction
The font size reduction from 17px to 16px might affect existing layouts and readability.
✅ Verification successful
Let me gather more specific information about the font size usage across these components.
Font size change is safe to proceed
The reduction from 17px to 16px is actually aligning with the existing design system, as multiple components already use 16px as their base font size. The codebase shows consistent usage of 16px, 14px, and smaller sizes (10px, 12px) for different components, with some components using relative units (rem) for better scaling. This change appears to be a standardization rather than a breaking modification.
- Most base-level text components already use 16px
- Components with specific requirements use either smaller sizes (14px, 12px, 10px) or relative units (rem)
- No components were found relying on 17px as a base for calculations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for components with specific pixel measurements that might be affected
echo "Components with pixel-based measurements:"
rg -l "(\d+)px" src/components/
Length of output: 2663
Script:
#!/bin/bash
# Search for specific font-size declarations in the codebase
echo "Font size declarations in components:"
rg "font-size:\s*\d+px" -A 1 --no-filename
# Check if there are any relative font sizes that might be affected
echo -e "\nRelative font size declarations:"
rg "font-size:\s*\d+(\.\d+)?rem" -A 1 --no-filename
# Check for any font-size CSS variables that might be using this as a base
echo -e "\nFont size CSS variables:"
rg ":root|--.*font" src/styles/
Length of output: 1050
package.json (2)
42-42: Verify @ably/ui breaking changes
The update from @ably/ui ^11.0.0 to ^15.1.0 is a major version bump that might introduce breaking changes.
16-16: Verify Node.js version compatibility with all dependencies
The Node.js version has been updated to ^20.13.1. This is a major version bump from ^18.12.0.
✅ Verification successful
Let me gather more information about the dependencies and their Node.js version requirements.
Let me check the workspace package.json file to ensure there are no conflicts.
Node.js 20.13.1 is compatible with all dependencies
After analyzing the dependencies' Node.js version requirements:
- All Gatsby-related packages require Node.js >=18.0.0
- Critical development tools like ESLint, TypeScript-ESLint support Node.js 20
- Testing libraries (@testing-library/react, jest) are compatible with Node.js 20
- No dependencies have explicit incompatibility with Node.js 20
The upgrade to Node.js 20.13.1 is safe to proceed with.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential compatibility issues with Node.js 20
# Check package.json for engines field in dependencies
echo "Checking dependencies for Node.js version constraints:"
jq -r '.dependencies | to_entries[] | select(.value | contains("engines"))' package.json
# Check for known compatibility issues in package-lock.json
if [ -f "package-lock.json" ]; then
echo "Checking package-lock.json for compatibility issues:"
jq -r '.packages | .[] | select(.engines != null) | select(.engines.node != null) | select(.engines.node | contains("20") | not)' package-lock.json
fi
Length of output: 240
Script:
#!/bin/bash
# Let's check the actual dependencies and their versions
echo "Package.json content:"
cat package.json
# Check if there's a yarn.lock file and show its content
if [ -f "yarn.lock" ]; then
echo -e "\nChecking yarn.lock for dependency constraints:"
cat yarn.lock | grep -A 1 "node "
fi
# List all package.json files in the project to check for workspace dependencies
echo -e "\nChecking for workspace package.json files:"
fd package.json
Length of output: 5749
Script:
#!/bin/bash
# Check the workspace package.json for any Node.js version constraints
echo "Checking how-tos/pub-sub/package.json for Node.js version constraints:"
cat how-tos/pub-sub/package.json
# Check if any dependencies explicitly require Node.js < 20
echo -e "\nChecking for dependencies that might have Node.js version constraints:"
for PKG in $(jq -r '.dependencies | keys[]' package.json); do
npm view $PKG engines --json 2>/dev/null
done
# Check the same for dev dependencies
for PKG in $(jq -r '.devDependencies | keys[]' package.json); do
npm view $PKG engines --json 2>/dev/null
done
Length of output: 11360
jamiehenson
force-pushed
the
web-3426-css-changes-to-modernize-docs
branch
from
35efe6e to
24e022a
Compare
jamiehenson
force-pushed
the
web-3426-css-changes-to-modernize-docs
branch
from
0f001e8 to
cc88cc2
Compare
jamiehenson
force-pushed
the
web-3426-css-changes-to-modernize-docs
branch
from
cc88cc2 to
61dabcd
Compare
jamiehenson
force-pushed
the
web-3426-css-changes-to-modernize-docs
branch
from
61dabcd to
23f86e9
Compare
fix: lighten font weights on product tiles fix: lighten font weights on parsed list item tags fix: fix conditional left property for sidebar fix: update font weight on inline code element fix: make top nav menu elements occupy full height fix: add ably ui types to local tsconfig fix: switch TW safelist config to array over object to satisfy v3 req chore: remove unneeded scroll-snap-margin-top chore: how to linting getaround fix: replace font-light with font-medium, add ui-text-p2 to table dl
jamiehenson
force-pushed
the
web-3426-css-changes-to-modernize-docs
branch
from
23f86e9 to
022e715
Compare
jamiehenson
approved these changes
Dec 5, 2024
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.
Description
This PR bumps
ably-uito the latest version (15.1.0at the time of writing), and with it solves various conflicts and regressions that come about via breaking changes introduced in later versions ofably-ui.Review
Instructions on how to review the PR.
Copilot Summary
This pull request includes a variety of updates and improvements across multiple files, focusing on dependency updates, configuration changes, and some refactoring of components and styles. The most important changes are summarized below:
Dependency and Configuration Updates:
.circleci/config.yml,.tool-versions, andpackage.json. [1] [2] [3]@ably/uidependency from version 11.0.0 to 15.1.0 inpackage.json.classnameswithclsxinpackage.json.@tailwindcss/container-queriesand updatedtailwindcssto version 3.3.6 inpackage.json. [1] [2]Codebase Refactoring:
ablySocialsection from the footer and refactored related components (Footer.tsx,FooterTopNav.tsx,FooterSocialMenu.tsx). [1] [2] [3] [4] [5]syntax-highlighterandsyntax-highlighter-registryinsetupTests.js.Style and Component Updates:
ui-btntoui-button-primaryandui-btn-secondarytoui-button-secondaryin multiple components (InArticleOptions.tsx,how-to.mdx). [1] [2]DropdownButton.tsxto addh-fullclass and bold the dropdown title. [1] [2]ContentLink.tsxandTitle.tsxto useui-text-p2andui-text-h4classes respectively. [1] [2]Documentation and Metadata Updates:
channels.yamland updated the Twitter handle format. [1] [2]README.md.These changes collectively improve the project's dependencies, simplify the codebase, and enhance the overall consistency and readability of the code.
Summary by CodeRabbit
New Features
FeatureLinkcomponent to replaceCallToActionfor enhanced call-to-action functionality.Bug Fixes
Documentation
Style
Chores
Tests