Improving comprehension epic - glossary tooltip setup #12236
Conversation
github-actions
bot
added
content 🖋️
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
hi Pablo, noticed few issues with the tooltip:
|
positioning and z-index issues |
pettinarip
changed the title
pettinarip
changed the title
pettinarip
changed the title
pettinarip
requested review from
wackerow,
corwintines,
minimalsm and
nhsz
as code owners
|
@konopkja nice catches. I'll create separate issues for those in order to keep this PR in the scope. |
pettinarip
changed the title
pettinarip
changed the title
There was a problem hiding this comment.
Not sure if this is in scope here, but noting the opacity makes this a little hard to read (from /get-eth page).
I don't think I'm seeing this same opacity issue on all cases though... this is the /dapps page and the text behind it doesn't bleed through like it does above.
Overall working well.. just left a few comments but otherwise think this is close to pulling in.
Good observation @wackerow. I'll take note of those styles differences since I'm going to tackle them once we finish the migration of the Tooltip component in #10974. That migration should fix most of the style issues but will double-check these ones as well. |
WalkthroughThe recent updates focus on enhancing localization and linking capabilities within the application. Key changes include the introduction of the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (5)
src/intl/en/glossary-tooltip.jsonis excluded by:!**/*.jsonsrc/intl/en/glossary.jsonis excluded by:!**/*.jsonsrc/intl/en/page-dapps.jsonis excluded by:!**/*.jsonsrc/intl/en/page-get-eth.jsonis excluded by:!**/*.jsonsrc/intl/en/page-layer-2.jsonis excluded by:!**/*.json
Files selected for processing (11)
- src/components/Glossary/GlossaryDefinition/index.tsx (2 hunks)
- src/components/Glossary/GlossaryTooltip/index.tsx (1 hunks)
- src/components/MdComponents.tsx (2 hunks)
- src/components/TooltipLink.tsx (3 hunks)
- src/components/Translation.tsx (1 hunks)
- src/layouts/Tutorial.tsx (2 hunks)
- src/lib/constants.ts (1 hunks)
- src/lib/utils/translations.ts (4 hunks)
- src/pages/dapps.tsx (6 hunks)
- src/pages/get-eth.tsx (2 hunks)
- src/pages/layer-2.tsx (4 hunks)
Additional comments: 21
src/components/Glossary/GlossaryTooltip/index.tsx (1)
- 16-24: The changes to the
GlossaryTooltipcomponent, specifically the addition of theoptionsprop with{ ns: "glossary-tooltip" }to theGlossaryDefinitioncomponent, are well-implemented. This allows for namespace customization, aligning with the PR's objectives to separate glossary tooltips from the main glossary content. The use ofuseBreakpointValuefrom Chakra UI for responsive design is also commendable.src/components/TooltipLink.tsx (1)
- 18-31: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [7-41]
The changes to the
TooltipLinkcomponent, including the renaming fromMdLinktoTooltipLink, making thehrefprop optional, and the conditional rendering logic to support glossary tooltips, are well-implemented. The use of a regex pattern to identify glossary links and the clear separation between glossary terms and regular links enhance the component's flexibility and functionality.src/components/Translation.tsx (1)
- 1-34: The changes to the
Translationcomponent, including the addition of thetransformprop and the merging logic for default and customtransformobjects, are well-implemented. These improvements enhance the component's flexibility and usability, supporting the seamless integration of glossary tooltips in translated content.src/components/Glossary/GlossaryDefinition/index.tsx (1)
- 1-29: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-48]
The changes to the
GlossaryDefinitioncomponent, including the addition of theoptionsprop for namespace customization and the override of theamapping to prevent nested tooltips, are well-implemented. The introduction ofDEFAULT_GLOSSARY_NSas a constant enhances maintainability. These improvements align with the PR's objectives and enhance the component's functionality and user experience.src/lib/constants.ts (1)
- 91-93: The addition of the
DEFAULT_GLOSSARY_NSconstant for the Glossary Definition Component is a good practice for consistency and maintainability. This change supports the PR's objectives by ensuring consistent namespace usage across the application.src/components/MdComponents.tsx (1)
- 27-35: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [32-151]
The changes in
MdComponents.tsx, including the removal ofMdLink, addition ofTooltipLink, and the update to thehtmlElementsobject to useTooltipLinkfor theaelement, are well-implemented. These changes align with the PR's objectives to adapt theTooltipLinkcomponent for broader use, including in markdown content, and enable the seamless integration of glossary tooltips in markdown-rendered content.src/layouts/Tutorial.tsx (1)
- 35-43: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [40-147]
The changes in
Tutorial.tsx, including the removal ofMdLink, addition ofTooltipLink, and the update to thetutorialsComponentsobject to useTooltipLinkfor theakey, are well-implemented. These changes align with the PR's objectives to adapt theTooltipLinkcomponent for broader use, including in tutorial content, and enable the seamless integration of glossary tooltips in tutorial-rendered content.src/lib/utils/translations.ts (2)
- 175-183: The addition of the "glossary-tooltip" namespace for specific paths (
/dapps/,/layer-2/,/wallets/,/get-eth/) is a logical extension to support the PR's objective of enhancing user comprehension with glossary tooltips. This change ensures that the necessary tooltip data is loaded for pages where tooltips are expected to be heavily utilized.- 215-215: Including the "glossary-tooltip" namespace for all layouts by default is a strategic decision that aligns with the incremental implementation approach of glossary tooltips across the website. This ensures that the tooltip functionality is readily available as new tooltips are added to various components and pages without requiring additional changes to the namespace requirements.
src/pages/get-eth.tsx (2)
- 31-31: The import of the
Translationcomponent is necessary for the subsequent change where a description is rendered using this component. This aligns with the PR's objective to enhance comprehension by utilizing glossary tooltips and dynamic translations.- 297-297: Changing the description rendering to use the
Translationcomponent with an ID (page-get-eth:page-get-eth-dex-desc) is a significant improvement. It allows for dynamic translations and the integration of glossary tooltips within the description text. This change enhances the user experience by providing contextual information and definitions for complex terms directly within the content.src/pages/layer-2.tsx (4)
- 291-293: The use of
<Translation />components for text content is a good practice for internationalization. This ensures that the content can be easily translated into different languages, enhancing accessibility and user comprehension across diverse audiences.- 454-456: The addition of the
<Translation />component for the DYOR (Do Your Own Research) section is consistent with the objective of making the site more accessible to a global audience. It's important to ensure that all dynamic content, such as links and buttons within translated text, is also appropriately handled to maintain functionality across languages.- 468-476: The use of
<Translation />components in the "Use Layer 2" section is appropriate. However, it's crucial to verify that all translation keys used here are correctly defined in the correspondingglossary-tooltip.jsonand other relevant namespaces. This ensures that the translations will load correctly without causing runtime errors or missing content.Verification successful
The executed scripts have successfully verified the presence of the translation keys
page-layer-2:layer-2-contract-accountsandpage-layer-2:layer-2-use-layer-2-generalized-1across multiple language files within thesrc/intl/directory. This confirms that the translation keys used in the "Use Layer 2" section are correctly defined in the corresponding translation files for various languages, ensuring that the translations will load correctly without causing runtime errors or missing content.Based on the provided output, it is evident that the translation keys are well-integrated across a wide range of language-specific JSON files, indicating a comprehensive localization effort for the "Use Layer 2" section. Therefore, the review comment concerning the verification of translation keys in the
glossary-tooltip.jsonand other relevant namespaces has been addressed through the verification process.* 646-648: The addition of the `` component in the FAQ section is a positive step towards making the content more accessible. Similar to previous comments, ensure that all translation keys are correctly defined and that the translations are accurate and contextually appropriate. This is especially important for FAQ content, which often addresses common questions and concerns.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that translation keys for the "Use Layer 2" section exist in the glossary-tooltip.json and other relevant namespaces. # This script is a placeholder and should be replaced with actual verification steps, such as checking the translation files or using a tool to verify the keys. echo "Verify translation keys in glossary-tooltip.json and other namespaces."Length of output: 150
Script:
#!/bin/bash # Search for the translation keys in glossary-tooltip.json rg "page-layer-2:layer-2-contract-accounts" -t json rg "page-layer-2:layer-2-use-layer-2-generalized-1" -t json # Assuming there might be a specific file for the "page-layer-2" namespace, search for it and then search within it fd "page-layer-2.*\.json" | xargs rg "layer-2-contract-accounts" fd "page-layer-2.*\.json" | xargs rg "layer-2-use-layer-2-generalized-1"Length of output: 24652
src/pages/dapps.tsx (6)
- 3-3: The addition of
ReactNodefrom "react" is appropriate for the changes made later in the file, whereReactNodeis used as a type for thedescriptionproperty in theCategoryinterface. This change aligns with the PR's objective to enhance comprehension through glossary tooltips, allowing for JSX elements (like<Translation />) to be used in descriptions.- 51-51: The import of
Translationfrom "@/components/Translation" is necessary for the implementation of glossary tooltips within the page content. This aligns with the PR's objectives and ensures that translations and glossary tooltips can be dynamically rendered based on the content.- 431-431: Changing the
descriptiontype in theCategoryinterface toReactNodeis a crucial update. This change supports the PR's goal of enhancing user comprehension by allowing glossary tooltips (and potentially other React components) to be included in category descriptions. It's a good practice to useReactNodefor properties that may contain complex JSX content.- 611-613: The replacement of text content with the
<Translation />component for thedescriptionof a benefit within theCategoryType.FINANCEcategory is a direct application of the PR's objectives. This change not only supports internationalization but also enables the integration of glossary tooltips within the text, enhancing user comprehension of complex terms. It's important to ensure that all instances where tooltips could enhance understanding are updated similarly.- 1346-1346: The use of the
<Translation />component for the subtitle under the "Get Started" section is consistent with the PR's aim to improve comprehension through glossary tooltips. This change ensures that the content is ready for internationalization and can include tooltips as needed. It's a good practice to review all static text content to identify additional opportunities for applying glossary tooltips.- 1868-1870: The use of the
<Translation />component for explaining how dApps work further aligns with the PR's objectives. This change ensures that the explanation is accessible to a wider audience through internationalization and can be enhanced with glossary tooltips to clarify complex concepts. Consistency in applying these changes across all educational content is key to maximizing user comprehension.
Description
This PR includes the initial setup to support glossary tooltips on pages other than md only pages.
The initial setup includes:
glossary-tooltip.jsonnamespace, which contains short versions of the terms seen in the glossary pageglossary.jsonnamespace is now only used by the glossary pageTooltipLinkcomponent to be used in MD pages and other components that use theTranslationcomponentAs part of this PR, we are adding new tooltips and term updates to the following pages:
Note: we are going to add more glossary tooltips to other pages in different PRs to make the review easier.
Summary by CodeRabbit
New Features
TooltipLinkcomponent to replaceMdLink, improving link handling across the application.dapps,get-eth,layer-2) to support internationalization.Refactor
GlossaryDefinition,GlossaryTooltip,Translation,Tutorial) to utilize newTooltipLinkfor better link management.Chores