-
Notifications
You must be signed in to change notification settings - Fork 11
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
Inject css and html element via JSX and plugin #122
Conversation
WalkthroughThe project has undergone a significant update focused on enhancing the documentation experience. A custom Marlowe theme for TypeDoc has been introduced, providing new styling and layout capabilities. The theme includes components for navigation, search, and layout, along with a new CSS file for visual appeal. Workflow adjustments ensure the theme is built and integrated during documentation generation, and editor configurations are set for consistent code formatting. 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's AI:
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files ignored due to filter (8)
- package.json
- theme/assets/favicon.ico
- theme/assets/github.svg
- theme/assets/logo.svg
- theme/package-lock.json
- theme/package.json
- theme/tsconfig.json
- typedoc.json
Files selected for processing (11)
- theme/.editorconfig (1 hunks)
- theme/.gitignore (1 hunks)
- theme/README.md (1 hunks)
- theme/assets/marlowe-theme.css (1 hunks)
- theme/src/MarloweTheme.tsx (1 hunks)
- theme/src/ThemeContext.tsx (1 hunks)
- theme/src/index.tsx (1 hunks)
- theme/src/templates/index.ts (1 hunks)
- theme/src/templates/layout.tsx (1 hunks)
- theme/src/templates/toolbar.tsx (1 hunks)
- theme/src/templates/utils.tsx (1 hunks)
Files skipped from review due to trivial changes (2)
- theme/.editorconfig
- theme/.gitignore
Additional comments: 15
theme/src/templates/index.ts (1)
- 1-2: Verify that the exported modules
toolbar
andlayout
are being utilized in the project to ensure that these exports are necessary.Verification successful
The search results indicate that the
toolbar
andlayout
components are indeed utilized within the project. Specifically, thelayout.tsx
andtoolbar.tsx
files export these components, andlayout.tsx
also makes a reference tocontext.toolbar(props)
, suggesting that thetoolbar
component is used within thelayout
component.Additionally, the
README.md
in thetheme
directory mentions the/templates
directory and references thetoolbar
as an example of an element that replaces the default theme, which implies that these components are part of the theme customization.The CSS file
marlowe-theme.css
contains styles that reference classes used within thetoolbar
component, further confirming its use in the project.Based on this information, it can be concluded that the exported
toolbar
andlayout
modules are indeed being utilized in the project, and their exports are necessary.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usage of exported modules 'toolbar' and 'layout'. rg --type tsx --type ts "from './templates/(toolbar|layout)'"Length of output: 97
Script:
#!/bin/bash # Search for usage of exported modules 'toolbar' and 'layout' without specifying file type. rg "from './templates/(toolbar|layout)'"Length of output: 48
Script:
#!/bin/bash # Search for usage of 'toolbar' and 'layout' modules in a broader scope without relying on the exact path. rg --type ts --type tsx --type js --type jsx "toolbar|layout"Length of output: 91
Script:
#!/bin/bash # Search for usage of 'toolbar' and 'layout' modules in all files. rg "toolbar|layout"Length of output: 2074
theme/src/templates/utils.tsx (1)
- 1-9: The
classNames
function is well-implemented, ensuring that only truthy keys are included in the class string and that extra classes are concatenated if provided.theme/src/MarloweTheme.tsx (1)
- 1-8: The
MarloweTheme
class correctly extendsDefaultTheme
and provides a custom render context. Ensure that theThemeContext
class is properly implemented to work with this custom theme.theme/src/ThemeContext.tsx (1)
- 1-25: The
ThemeContext
class correctly extendsDefaultThemeRenderContext
. Theinit
method dynamically binds template functions to the context, which is a flexible approach to handle rendering. Ensure that all templates are correctly implemented and that the keys intemplates
match the method names inThemeContext
.theme/README.md (1)
- 1-17: The README instructions are clear and provide a straightforward guide for building the theme and previewing changes. Ensure that the
typedoc.json
configuration file is properly set up to use the new theme.theme/src/templates/toolbar.tsx (1)
- 1-52: The
toolbar
template is well-structured and includes essential elements like search, navigation links, and branding. Verify that thenavigationLinks
option is properly configured in the TypeDoc options and that the assets are correctly placed in theassets/marlowe
directory.Verification successful
The
navigationLinks
option is confirmed to be set in the TypeDoc options. However, there was a syntax error with thefd
command used to check the assets. I will correct the command and rerun the script to verify the presence of thegithub.svg
andlogo.svg
assets.
The specified path for the assets does not appear to be a directory. I will correct the
fd
command to search for the assets without specifying a path to ensure we locate them wherever they may be in the project.
The assets
github.svg
andlogo.svg
have been successfully located within the project:
github.svg
is located in./theme/assets/
.logo.svg
is found in two locations:./doc/image/
and./theme/assets/
.This confirms that the assets are present and likely correctly placed for use in the
toolbar.tsx
template, assuming that thetheme/assets/
directory is the correct location referenced in the code.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the navigationLinks option is set in TypeDoc options. rg --type json '"navigationLinks":' typedoc.json # Check that the assets are correctly placed. fd --type f --name 'github.svg' --exec ls -l {} fd --type f --name 'logo.svg' --exec ls -l {}Length of output: 673
Script:
#!/bin/bash # Correct the fd command to check for the presence of the assets. fd 'github.svg' theme/assets/marlowe --exec ls -l {} fd 'logo.svg' theme/assets/marlowe --exec ls -l {}Length of output: 323
Script:
#!/bin/bash # Search for the assets without specifying a path. fd 'github.svg' --exec ls -l {} fd 'logo.svg' --exec ls -l {}Length of output: 252
theme/src/index.tsx (1)
- 1-75: The plugin setup in
index.tsx
includes asset copying and HTML modifications. Ensure that the asset paths are correct and that the JavaScript injected into thebody.begin
andbody.end
hooks does not introduce any security vulnerabilities.Verification successful
The previous script contained errors. I will correct these and run a new script to verify the asset paths and inspect the JavaScript code for potential security vulnerabilities.
The verification of the asset paths has been successful:
favicon.ico
is located at./theme/assets/favicon.ico
.marlowe-theme.css
is located at./theme/assets/marlowe-theme.css
.The presence of
<script>
tags intheme/src/index.tsx
has been confirmed, but the content within these tags needs to be reviewed to ensure there are no security vulnerabilities. Since the output does not show the JavaScript content, I will need to run another script to extract and review the JavaScript code within the<script>
tags.
The script output provides the JavaScript code within the
<script>
tags fromtheme/src/index.tsx
. Here's the analysis:
The first script tag contains:
console.log(`Loaded ${location.href}`);This is a simple console log statement that outputs the current URL. There are no apparent security vulnerabilities with this line of code.
The second script tag is not fully shown in the output, but it starts with a
try
block and selects an element with the class.tsd-generator a
. Without the full context of the script, it's not possible to definitively assess security implications.To ensure a thorough review, I will need to extract the full content of the second script tag to analyze it for potential security vulnerabilities.
The full content of the second
<script>
tag fromtheme/src/index.tsx
has been extracted. Here's the analysis:The script is creating a link element and inserting it after an element with the class
.tsd-generator a
. The link points tohttps://github.com/input-output-hk/marlowe-ts-sdk
, opens in a new tab, and hasrel="noreferrer"
to prevent the browser from sending the current address as a referrer. The script is wrapped in atry
block to handle any potential errors silently.From the provided code, there are no obvious security vulnerabilities such as:
- Direct use of user input in the DOM which could lead to Cross-Site Scripting (XSS) attacks.
- External resource URLs that are hardcoded and do not rely on user input.
However, it's important to note that the silent catch block (
catch (error) {}
) is not logging or handling the error, which could make debugging difficult if the script fails.In conclusion, based on the provided context, the JavaScript code within the
<script>
tags does not introduce any security vulnerabilities. The asset paths have been verified and are correct.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the asset paths are correct. fd --type f --name 'favicon.ico' --exec ls -l {} fd --type f --name 'marlowe-theme.css' --exec ls -l {} # Check for potential security vulnerabilities in the injected JavaScript. rg --type js "<script>" src/index.tsxLength of output: 703
Script:
#!/bin/bash # Verify that the asset paths are correct. fd 'favicon.ico' --exec ls -l {} fd 'marlowe-theme.css' --exec ls -l {} # Check for potential security vulnerabilities in the injected JavaScript. rg "<script>" theme/src/index.tsxLength of output: 276
Script:
#!/bin/bash # Extract and review the JavaScript code within the <script> tags for potential security vulnerabilities. rg --context 5 "<script>" theme/src/index.tsxLength of output: 603
Script:
#!/bin/bash # Extract the full content of the second <script> tag to analyze for potential security vulnerabilities. rg --after-context 30 "<script>" theme/src/index.tsx | tail -n +7Length of output: 1485
theme/src/templates/layout.tsx (1)
- 1-83: The
layout
template correctly sets up the HTML structure for the documentation pages, including meta tags, stylesheets, and scripts. Verify that all linked assets exist and that the custom CSS option is properly handled.theme/assets/marlowe-theme.css (7)
127-140: The media query for
prefers-color-scheme: light
is correctly set up to override the root variables for users who have indicated a preference for a light theme. This is a good practice for supporting system-level theme preferences.143-156: Similarly, the media query for
prefers-color-scheme: dark
is well-implemented. It's important to test these styles across different devices and browsers to ensure the theme switches correctly.205-212: The input and select styles are consistent and use the root variables for styling, which is good for maintainability. However, verify that the styles are applied correctly across different browsers, as form elements can be rendered differently.
228-232: The focus styles for input and select elements are important for accessibility. Ensure that the contrast ratio for the border color meets the WCAG guidelines.
306-317: The preformatted text block styles are set correctly. It's important to ensure that the
--pre-color-background
variable provides sufficient contrast for readability.334-356: The button styles are well-defined and use CSS variables for easy theming. However, ensure that the hover state is distinguishable from the default state for better user experience.
Verification successful
The executed script confirms that the
button:hover
state in themarlowe-theme.css
file is defined with different CSS variables for color, background, and border-color properties. This suggests that the hover state should indeed be distinguishable from the default state, as the variables imply a change in these properties on hover.Based on the information provided:
- The hover state for buttons is styled with different variables for color, background, and border-color.
- The use of separate variables (
--button-color-hover
,--button-background-hover
,--button-border-color-hover
) indicates that the theme is designed to have a hover state that is visually distinct from the default state.Given this, the review comment has been addressed in the code, and the hover state should be distinguishable from the default state, assuming the variables are defined with contrasting values.
* 860-902: The responsive design media queries are correctly targeting specific device widths. It's crucial to test these styles on actual devices to ensure the layout adjusts as expected.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Ensure button hover states are distinguishable ast-grep --lang css --pattern $'button:hover { $$$ }'Length of output: 388
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- .github/workflows/deploy-docs.yaml (1 hunks)
- .github/workflows/test-suite.yaml (1 hunks)
Files skipped from review due to trivial changes (2)
- .github/workflows/deploy-docs.yaml
- .github/workflows/test-suite.yaml
Pending linter fixes - let me know if there's any feedback for changes. If needed, there's a path to get some design support as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (8)
- theme/assets/marlowe-theme.css (1 hunks)
- theme/src/MarloweTheme.tsx (1 hunks)
- theme/src/ThemeContext.tsx (1 hunks)
- theme/src/index.tsx (1 hunks)
- theme/src/templates/index.ts (1 hunks)
- theme/src/templates/layout.tsx (1 hunks)
- theme/src/templates/toolbar.tsx (1 hunks)
- theme/src/templates/utils.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (6)
- theme/assets/marlowe-theme.css
- theme/src/ThemeContext.tsx
- theme/src/index.tsx
- theme/src/templates/index.ts
- theme/src/templates/toolbar.tsx
- theme/src/templates/utils.tsx
Additional comments: 4
theme/src/MarloweTheme.tsx (1)
- 4-8: Ensure that the
ThemeContext
constructor properly handles the argumentsthis
,pageEvent
, andthis.application.options
. If these arguments have changed or their handling inThemeContext
is complex, it might be worth adding a test case to verify correct behavior.Verification successful
To verify the handling of arguments in the
ThemeContext
constructor, we need to locate the definition of theThemeContext
class and review how the constructor processes the arguments. Let's write a script to find theThemeContext
class definition.
Let's try a broader search pattern to locate the
ThemeContext
class definition usingast-grep
. If this fails, we will switch to usingrg
to search for the class definition in the codebase.
To locate the
ThemeContext
class definition, we will now userg
to search for the string "class ThemeContext" in the codebase.
The
ThemeContext
class is defined in the filetheme/src/ThemeContext.tsx
. To verify the handling of the constructor arguments, we need to examine the constructor within this file.
The
ThemeContext
constructor intheme/src/ThemeContext.tsx
takes three arguments:theme
,page
, andoptions
. It calls thesuper
constructor ofDefaultThemeRenderContext
with these arguments and then calls aninit
method.The arguments
theme
,page
, andoptions
correspond tothis
,pageEvent
, andthis.application.options
from theMarloweTheme
class'sgetRenderContext
method. The constructor appears to be handling the arguments correctly by passing them to thesuper
constructor.Based on the provided output, the
ThemeContext
constructor is set up to handle the arguments as expected in the review comment. No issues have been found with the constructor's handling of the arguments.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for ThemeContext constructor usage to ensure it's correctly handling the new arguments. ast-grep --lang typescript --pattern $'new ThemeContext($_, $_, $_)'Length of output: 69
Script:
#!/bin/bash # Find the file that contains the definition of the ThemeContext class. ast-grep --lang typescript --pattern $'class ThemeContext { $$$ }'Length of output: 67
Script:
#!/bin/bash # Broaden the search to find any class that contains a constructor. ast-grep --lang typescript --pattern $'class $_ { constructor($$$) { $$$ } }'Length of output: 78
Script:
#!/bin/bash # Use rg to find the file containing the ThemeContext class definition. rg "class ThemeContext"Length of output: 114
Script:
#!/bin/bash # Display the contents of ThemeContext.tsx to review the constructor. cat theme/src/ThemeContext.tsxLength of output: 677
theme/src/templates/layout.tsx (3)
- 11-101: Verify that the
classNames
utility function is correctly applied and that the CSS classes it references (container
,container-main
) are defined in the CSS files. This is important for ensuring that the layout renders correctly.Verification successful
The output from the shell scripts indicates that the CSS classes
container
andcontainer-main
are indeed defined in themarlowe-theme.css
file. However, the search for theclassNames
function usage failed because the file typetypescript
was not recognized byrg
. We should correct the file type and rerun the search to verify the correct application of theclassNames
function.Let's correct the script to search for the
classNames
function usage without specifying the file type, as the function could be in a.tsx
file which is a TypeScript file with JSX syntax.
The output confirms that the
classNames
utility function is defined inutils.tsx
and is imported and used inlayout.tsx
. Additionally, the CSS classescontainer
andcontainer-main
are defined in themarlowe-theme.css
file. Therefore, theclassNames
utility function is correctly applied, and the CSS classes it references are indeed defined.* 17-59: The usage of `context.relativeURL` in script and link tags is correct and follows standard practices for TypeDoc themes.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the classNames function usage and ensure it's correctly applied. rg --type typescript 'classNames' # Search for the CSS classes in the stylesheets to ensure they are defined. rg --type css 'container|container-main'Length of output: 354
Script:
#!/bin/bash # Search for the classNames function usage and ensure it's correctly applied. rg 'classNames'Length of output: 203
- 17-26: The meta tags for responsive design, charset, and IE compatibility are set up correctly, following best practices for web development.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work 🙌,
A couple of notes:
- Please put the
theme
folder inside thedoc
folder so we know this is the theme for the docs. - We have some issues with wide monitors (attaching screenshot)
- I believe the code blocks looked better in the previous theme as it had more contrast with the background. Maybe we can change the code highlight to use a background with more contrast?
New theme
Old theme
.github/workflows/test-suite.yaml
Outdated
@@ -35,7 +35,7 @@ jobs: | |||
|
|||
- name: 🏥 Check docs generation | |||
run: | | |||
nix develop --show-trace --command bash -c "npm run docs" | |||
nix develop --show-trace --command bash -c "npm --prefix ./theme/ run build && npm run docs" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have this theme folder inside of doc/theme
? Also, notice there is an image folder there. If we could reutilize those assets it would be awesome
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing more to add than what has been discussed...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to filter (7)
- doc/image/favicon.ico
- doc/image/github.svg
- doc/image/logo-header.svg
- doc/theme/package-lock.json
- doc/theme/package.json
- doc/theme/tsconfig.json
- typedoc.json
Files selected for processing (14)
- .github/workflows/deploy-docs.yaml (1 hunks)
- .github/workflows/test-suite.yaml (1 hunks)
- doc/theme/.editorconfig (1 hunks)
- doc/theme/.gitignore (1 hunks)
- doc/theme/README.md (1 hunks)
- doc/theme/assets/marlowe-theme.css (1 hunks)
- doc/theme/src/MarloweTheme.tsx (1 hunks)
- doc/theme/src/ThemeContext.tsx (1 hunks)
- doc/theme/src/index.tsx (1 hunks)
- doc/theme/src/templates/footer.tsx (1 hunks)
- doc/theme/src/templates/index.ts (1 hunks)
- doc/theme/src/templates/layout.tsx (1 hunks)
- doc/theme/src/templates/toolbar.tsx (1 hunks)
- doc/theme/src/templates/utils.tsx (1 hunks)
Files skipped from review due to trivial changes (4)
- .github/workflows/deploy-docs.yaml
- doc/theme/.editorconfig
- doc/theme/.gitignore
- doc/theme/src/templates/index.ts
Additional comments: 22
doc/theme/src/templates/utils.tsx (1)
- 1-11: The
classNames
function implementation looks correct and efficient. It filters the keys based on the truthiness of their values, concatenates them with any additional CSS classes provided, and ensures there are no extra spaces.doc/theme/src/MarloweTheme.tsx (1)
- 1-7: The
MarloweTheme
class correctly extendsDefaultTheme
and overridesgetRenderContext
to return an instance ofThemeContext
. This is a standard pattern for creating custom themes in TypeDoc.doc/theme/src/ThemeContext.tsx (1)
- 1-28: The
ThemeContext
class extendsDefaultThemeRenderContext
and initializes by binding template functions to itself. Ensure that all template functions intemplates
are compatible with theThemeContext
class.Verification successful
The
rg
command has successfully found references toThemeContext
in the codebase. TheThemeContext
class is extended fromDefaultThemeRenderContext
and is used in theMarloweTheme.tsx
file to override thegetRenderContext
method, which returns a new instance ofThemeContext
.The compatibility of the template functions with the
ThemeContext
class cannot be fully verified without seeing the actual implementation of the functions within thetemplates
module. However, the usage pattern inThemeContext.tsx
suggests that the functions are expected to be compatible since they are being bound to instances ofThemeContext
.Based on the information available, there are no obvious issues with the references to
ThemeContext
in the provided context. For a complete verification, one would need to review the actual implementation of the functions in thetemplates
module to ensure they are indeed compatible when bound toThemeContext
.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that all functions in the templates module are compatible with ThemeContext. # Expecting a list of functions and their compatibility status. ast-grep --lang typescript --pattern $'export function $_($$$) { $$$ }' --exec 'rg -C3 --type typescript "ThemeContext"'Length of output: 315
Script:
#!/bin/bash # Find all exported functions in the `templates` module. ast-grep --lang typescript --pattern $'export function $_($$$) { $$$ }' # Then, check for references to `ThemeContext` within the TypeScript files to see how the functions are used. rg --type typescript "ThemeContext"Length of output: 20439
Script:
#!/bin/bash # Correct the ripgrep command to search for references to `ThemeContext` without specifying the file type. rg "ThemeContext"Length of output: 502
doc/theme/README.md (1)
- 1-17: The README.md provides clear instructions for building and using the Marlowe Typedoc Theme. It's important to ensure that the
typedoc.json
file is correctly configured to recognize the/dist
directory as the plugin location.Verification successful
The attempt to verify the configuration of
typedoc.json
to use the/dist
directory as the plugin location failed because the filetypedoc.json
could not be found in the./doc
directory. This suggests that either the file is located in a different directory or it does not exist in the repository.To proceed, I will search for the
typedoc.json
file in the repository to verify its location and contents.
The
typedoc.json
file was found at the root of the repository, among other locations within various packages. To verify the plugin configuration, I will now examine the contents of the roottypedoc.json
file to check if it is configured to use the/dist
directory as the plugin location.
The
typedoc.json
file at the root of the repository is indeed configured to use the./doc/theme/dist/index.js
file as the plugin, which aligns with the instructions provided in theREADME.md
for the Marlowe Typedoc Theme. This confirms that the plugin location is set correctly in the configuration file.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that typedoc.json is configured to use the /dist directory as the plugin location. # Expecting the content of typedoc.json showing the plugin configuration. cat ./doc/typedoc.json | jq '.plugin'Length of output: 88
Script:
#!/bin/bash # Search for the typedoc.json file in the repository to verify its location. fd typedoc.jsonLength of output: 366
Script:
#!/bin/bash # Check the contents of the root typedoc.json file for the plugin configuration. cat typedoc.json | jq '.plugin'Length of output: 65
.github/workflows/test-suite.yaml (1)
- 38-38: The addition of
npm --prefix ./doc/theme/ run build
in the "Check docs generation" step ensures that the theme is built before generating the docs. This is a necessary step given the changes in the PR.doc/theme/src/templates/toolbar.tsx (1)
- 1-74: The toolbar template includes a search field, navigation links, and a logo with the project version. Ensure that the search functionality is properly implemented and that the navigation links are correctly populated from the
context.options
.doc/theme/src/index.tsx (1)
- 1-77: The
index.tsx
file correctly sets up the plugin and defines theMarloweTheme
. ThecopySync
function is used to copy assets to the output directory. Ensure that the paths used in the function are correct and that it handles all necessary file types.doc/theme/src/templates/layout.tsx (1)
- 1-101: The layout template is well-structured and makes use of the
classNames
utility for conditional class assignment. Ensure that the classes used are defined in the CSS and that the layout is responsive.Verification successful
It appears there was a syntax error in the previous script. The
--exec
flag is not a valid option forrg
. Instead, I will correct the script to userg
properly to search for the CSS class definitions within themarlowe-theme.css
file.
The script attempted to search for CSS class definitions within a file that does not exist at the specified path. To correct this, I will adjust the script to first locate the
marlowe-theme.css
file usingfd
and then search for the CSS class definitions within that file.
The output confirms that the CSS classes used in the
layout.tsx
file are defined within themarlowe-theme.css
file. The classes.container
,.container-main
,.col-content
,.col-sidebar
,.page-menu
, and.site-menu
are present, which suggests that the layout should be styled as intended by the theme.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the classes used in the layout are defined in the CSS. # Expecting the CSS rules for the classes used in the layout. rg --type css 'container|container-main|col-content|col-sidebar|page-menu|site-menu' --exec 'cat ./doc/theme/src/marlowe-theme.css'Length of output: 542
Script:
#!/bin/bash # Search for the CSS class definitions used in the layout within the marlowe-theme.css file. rg --type css 'container|container-main|col-content|col-sidebar|page-menu|site-menu' ./doc/theme/src/marlowe-theme.cssLength of output: 194
Script:
#!/bin/bash # Locate the marlowe-theme.css file and search for the CSS class definitions used in the layout. fd 'marlowe-theme.css' --exec rg 'container|container-main|col-content|col-sidebar|page-menu|site-menu'Length of output: 425
doc/theme/assets/marlowe-theme.css (14)
1-7: The CSS reset rules applied here are comprehensive and should ensure a consistent starting point across different browsers.
68-133: The CSS custom properties (variables) defined for light and dark themes are well-organized and follow a consistent naming convention. However, the use of
var(--color-background-secondary)
in--pre-color-background
and--code-color-background
without defining--color-background-secondary
could lead to issues if the variable is not defined elsewhere.135-148: The media query for
prefers-color-scheme: light
correctly applies the light theme variables. This is a good practice for supporting automatic theme changes based on the user's system preferences.151-164: Similarly, the media query for
prefers-color-scheme: dark
is well-implemented, ensuring that users with a preference for dark mode will see the appropriate theme.167-193: The use of
data-theme
attribute selectors to apply theme-specific styles is a good approach for theme switching via JavaScript. It allows for dynamic theme changes without relying on media queries.195-198: The body font-family and line-height are set globally, which is a common practice for maintaining consistency across the site.
210-213: The styles applied to SVG elements ensure they align properly with text and do not shrink when used as flex children, which is important for visual consistency.
215-222: The input and select elements have consistent font-family, size, and border styles, which is good for a uniform look across form elements.
238-242: The focus styles for input and select elements are clear and accessible, which is important for usability.
253-259: The
.tsd-typography:empty
selector is used to hide any typography elements that are empty. This can prevent layout issues caused by empty elements taking up space.261-277: The heading styles within
.tsd-typography
are uniform, but the font-size is set to1rem
for all heading levels, which may not be intentional as it makes all headings the same size. This should be reviewed to ensure it matches the design requirements.320-339: The code block and inline code styles are consistent with the theme and use the monospace font family, which is standard for displaying code.
874-896: The media query for max-width 769px adjusts font sizes and spacing for mobile devices, which is important for responsive design. However, the
--anchor-icon-offset
and--anchor-icon-size
are adjusted, which may need verification to ensure they align correctly with the design on mobile devices.944-961: The media query for a minimum width of 1200px sets the max-height and overflow properties for navigation menus, ensuring they are sticky and scrollable, which is a good practice for large screens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (7)
- doc/image/favicon.ico
- doc/image/github.svg
- doc/image/logo-header.svg
- doc/theme/package-lock.json
- doc/theme/package.json
- doc/theme/tsconfig.json
- typedoc.json
Files selected for processing (14)
- .github/workflows/deploy-docs.yaml (1 hunks)
- .github/workflows/test-suite.yaml (1 hunks)
- doc/theme/.editorconfig (1 hunks)
- doc/theme/.gitignore (1 hunks)
- doc/theme/README.md (1 hunks)
- doc/theme/assets/marlowe-theme.css (1 hunks)
- doc/theme/src/MarloweTheme.tsx (1 hunks)
- doc/theme/src/ThemeContext.tsx (1 hunks)
- doc/theme/src/index.tsx (1 hunks)
- doc/theme/src/templates/footer.tsx (1 hunks)
- doc/theme/src/templates/index.ts (1 hunks)
- doc/theme/src/templates/layout.tsx (1 hunks)
- doc/theme/src/templates/toolbar.tsx (1 hunks)
- doc/theme/src/templates/utils.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (14)
- .github/workflows/deploy-docs.yaml
- .github/workflows/test-suite.yaml
- doc/theme/.editorconfig
- doc/theme/.gitignore
- doc/theme/README.md
- doc/theme/assets/marlowe-theme.css
- doc/theme/src/MarloweTheme.tsx
- doc/theme/src/ThemeContext.tsx
- doc/theme/src/index.tsx
- doc/theme/src/templates/footer.tsx
- doc/theme/src/templates/index.ts
- doc/theme/src/templates/layout.tsx
- doc/theme/src/templates/toolbar.tsx
- doc/theme/src/templates/utils.tsx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (7)
- doc/image/favicon.ico
- doc/image/github.svg
- doc/image/logo-header.svg
- doc/theme/package-lock.json
- doc/theme/package.json
- doc/theme/tsconfig.json
- typedoc.json
Files selected for processing (14)
- .github/workflows/deploy-docs.yaml (1 hunks)
- .github/workflows/test-suite.yaml (1 hunks)
- doc/theme/.editorconfig (1 hunks)
- doc/theme/.gitignore (1 hunks)
- doc/theme/README.md (1 hunks)
- doc/theme/assets/marlowe-theme.css (1 hunks)
- doc/theme/src/MarloweTheme.tsx (1 hunks)
- doc/theme/src/ThemeContext.tsx (1 hunks)
- doc/theme/src/index.tsx (1 hunks)
- doc/theme/src/templates/footer.tsx (1 hunks)
- doc/theme/src/templates/index.ts (1 hunks)
- doc/theme/src/templates/layout.tsx (1 hunks)
- doc/theme/src/templates/toolbar.tsx (1 hunks)
- doc/theme/src/templates/utils.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (13)
- .github/workflows/deploy-docs.yaml
- doc/theme/.editorconfig
- doc/theme/.gitignore
- doc/theme/README.md
- doc/theme/assets/marlowe-theme.css
- doc/theme/src/MarloweTheme.tsx
- doc/theme/src/ThemeContext.tsx
- doc/theme/src/index.tsx
- doc/theme/src/templates/footer.tsx
- doc/theme/src/templates/index.ts
- doc/theme/src/templates/layout.tsx
- doc/theme/src/templates/toolbar.tsx
- doc/theme/src/templates/utils.tsx
Additional comments: 1
.github/workflows/test-suite.yaml (1)
- 38-38: The command to build the documentation theme and generate docs is correctly scoped to the respective directories. Ensure that the
npm run build
command within thedoc/theme/
directory and thenpm run docs
command at the root are configured correctly to work with this setup.Verification successful
The
npm run build
command in thedoc/theme/
directory is set to"tsc"
, which indicates it is using TypeScript Compiler to build the theme. Thenpm run docs
command in the rootpackage.json
is set to"typedoc . --treatWarningsAsErrors --options ./typedoc.json"
, which is the command to generate the documentation using TypeDoc with the specified options.Both commands appear to be correctly set up for their respective tasks.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the npm run build command is set up correctly in the doc/theme/ directory cat doc/theme/package.json | jq '.scripts.build' # Verify that the npm run docs command is set up correctly in the root package.json cat package.json | jq '.scripts.docs'Length of output: 153
@nhenin Added the footer per discussion on Slack. Still bare bones because don't want to mess with more dark theme work atm, but can continue to iterate. @hrajchert Tweaked contrast a bit for code blocks. Let me know if it needs to be more aggressive and I can pull in highlight.js instead |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! Just made a small modification to the max width and added a scriv entry
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- changelog.d/20240105_164037_hrajchert_plt_8836.md (1 hunks)
- doc/theme/assets/marlowe-theme.css (1 hunks)
Files skipped from review due to trivial changes (1)
- changelog.d/20240105_164037_hrajchert_plt_8836.md
Additional comments: 13
doc/theme/assets/marlowe-theme.css (13)
1-7: The CSS reset rules applied here are comprehensive and should ensure a consistent baseline across browsers. However, it's important to test these rules across different browsers to ensure they don't cause any unexpected layout issues.
68-133: The CSS custom properties (variables) defined for light and dark themes are well-organized. Ensure that these color values align with the design specifications for the Marlowe theme.
135-149: The media query for
prefers-color-scheme: light
correctly applies the light theme variables. It's good practice to ensure that the theme respects user preferences for light or dark mode.151-165: Similarly, the media query for
prefers-color-scheme: dark
is well-implemented. Testing on various devices and browsers is recommended to ensure the dark mode is triggered as expected.167-193: The use of
data-theme
attribute selectors to apply theme-specific styles is a good approach for theme switching. Make sure that the JavaScript logic that toggles these attributes is robust and handles user preferences correctly.195-198: The body font-family and line-height are set globally, which is a good practice for consistency. Verify that the font stack is cross-platform and that all fonts fall back gracefully.
210-213: The styles applied to SVG elements ensure they align correctly with text and do not shrink. This is important for icons and other decorative SVGs used within the UI.
215-222: Input and select elements have a consistent font-family and border styles. It's important to check the accessibility of these elements, especially the contrast of the border color against the background.
238-242: The focus styles for input and select elements are crucial for accessibility. Ensure that the focus state is clearly visible and meets WCAG contrast requirements.
874-896: The responsive design adjustments in the media query for max-width 769px are important for mobile usability. Test on various devices to ensure the UI scales down appropriately and remains usable.
923-931: The sticky positioning for the sidebar in the media query for widths between 770px and 1399px is a good feature for desktop usability. Verify that the sticky behavior works as expected across different browsers.
944-961: The media query for min-width 1200px ensures that the navigation and site menu are sticky and have a maximum height. This is a good UX practice for desktop users, allowing easy navigation regardless of scroll position.
966-969: The
.tsd-generator
class has a padding-top set to 0 and max-height to none. Verify that this does not cause any visual regression or layout shift, especially when the content is dynamically generated.
This PR implements a theme for typedoc through the plugin system without the need for an npm module. Build the theme via
npm run build
inside thetheme/
directory then rebuild from the root projectnpm run docs
.Summary by CodeRabbit
New Features
Documentation
Chores
.editorconfig
and.gitignore
files for consistent coding styles and Git usage.CI/CD Updates