-
Notifications
You must be signed in to change notification settings - Fork 27
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
Outline Fall/Winter Update!! #426
Conversation
…utline-core-button
…utline-core-button
…utline-core-button
…tylesheets-controller.
🦋 Changeset detectedLatest commit: 3ce056d The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for outlinejs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
WalkthroughThis series of changes updates various components within a project, focusing on enhancing core components like links, alerts, and buttons. It involves updates to component properties, method signatures, styling, and documentation to reflect these enhancements. Additionally, configuration files for Storybook, ESLint, PostCSS, and Yarn are updated to support these changes. The effort aims to streamline component usage, improve documentation for developers, and ensure consistency across the project's UI components. Changes
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 (
|
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
Files ignored due to filter (4)
- package.json
- packages/components/outline-core-link/package.json
- packages/components/outline-core-link/yarn.lock
- yarn.lock
Files selected for processing (12)
- packages/components/@deprecated/outline-form/src/outline-form.global.scoped.css (1 hunks)
- packages/components/outline-core-contained-element.ts (1 hunks)
- packages/components/outline-core-link/index.ts (1 hunks)
- packages/components/outline-core-link/src/outline-core-link.ts (4 hunks)
- packages/components/outline-core-link/src/style/outline-core-link.css (1 hunks)
- packages/components/outline-core-link/src/style/outline-core-link.lightDom.css (1 hunks)
- packages/components/outline-core-link/src/style/outline-core-link.vars.css (1 hunks)
- packages/documentation/outline-storybook/config/storybook.main.css (3 hunks)
- packages/documentation/outline-storybook/stories/components/outline-core-link.stories.mdx (1 hunks)
- packages/outline-templates/default/postcss.config.js (1 hunks)
- packages/tools/outline-config/postcss.config.js (1 hunks)
- postcss.config.js (1 hunks)
Files skipped from review due to trivial changes (6)
- packages/components/@deprecated/outline-form/src/outline-form.global.scoped.css
- packages/components/outline-core-link/index.ts
- packages/components/outline-core-link/src/style/outline-core-link.css
- packages/components/outline-core-link/src/style/outline-core-link.vars.css
- packages/documentation/outline-storybook/stories/components/outline-core-link.stories.mdx
- postcss.config.js
Additional comments (Suppressed): 9
packages/components/outline-core-link/src/style/outline-core-link.lightDom.css (1)
- 1-28: The CSS variables used in this hunk are not defined within this file. Ensure that they are defined in the imported
outline-core-link.vars.css
file or elsewhere in the project. Also, verify that the@nested-import
directive is correctly processed by your PostCSS configuration, as it's not a standard CSS feature.packages/outline-templates/default/postcss.config.js (1)
- 1-8: The PostCSS configuration has been updated. The
postcss-import
,postcss-nested
,postcss-custom-properties
, andautoprefixer
plugins have been removed. They are replaced withpostcss-nested-import
andpostcss-preset-env
. Ensure that these changes do not affect the stylesheets' compatibility with different browsers and that the nested imports work as expected. Also, verify that the removal ofpostcss-custom-properties
does not affect the usage of CSS custom properties in your stylesheets.packages/tools/outline-config/postcss.config.js (1)
- 1-8: The changes in the PostCSS configuration seem to be in line with the summary provided. The
postcss-import
plugin has been replaced withpostcss-nested-import
, and thepostcss-preset-env
plugin has been added. Thepostcss-nested
,postcss-custom-properties
, andautoprefixer
plugins have been removed.Ensure that these changes do not break the existing stylesheets and that they are compatible with the rest of the codebase. The
postcss-preset-env
plugin includesautoprefixer
andpostcss-custom-properties
by default, so removing them explicitly should not cause any issues. However, thepostcss-nested-import
plugin behaves differently thanpostcss-import
, so verify that the import statements in your stylesheets are compatible with this change.- require('postcss-import'), - require('postcss-nested'), - require('postcss-custom-properties'), - require('autoprefixer'), + require('postcss-nested-import'), + require('postcss-preset-env')({ stage: 1 }),packages/documentation/outline-storybook/config/storybook.main.css (2)
1-133: The new hunk is importing styles from a new location and introduces a new set of styles for various HTML elements. The changes seem to be consistent with the old hunk, with the exception of the removal of a large block of CSS variables. Ensure that the removal of these variables does not affect the styling of other components that might be using them. Also, verify that the new import path is correct and accessible.
141-177: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [141-194]
The new hunk introduces styles for form elements, media elements, and the
[hidden]
attribute. The changes are consistent with the old hunk. However, ensure that these new styles do not conflict with existing styles in other parts of the application.packages/components/outline-core-link/src/outline-core-link.ts (3)
1-15: The import statements have been updated to include new stylesheets and the
AdoptedStyleSheets
class. Ensure that these new files and classes exist and are accessible from this location.33-38: The
OutlineCoreLink
class now includes thecomponentVars
stylesheet in its styles and has a newadoptedStylesheets
property. Ensure that thecomponentVars
stylesheet is correctly defined and that theadoptedStylesheets
property is used correctly throughout the class.196-204: The
fullMarkupInSlot
method has been updated to remove the call toisValidTopLevelSlottedLink
. This might affect the behavior of the method, as it no longer checks if the component has a single<a>
tag as a child of the default slot. Ensure that this change is intentional and does not introduce bugs.packages/components/outline-core-contained-element.ts (1)
- 1-88: The new class
OutlineCoreContainedElement
extendsOutlineElement
and introduces properties for padding and margin. It also includes a methodupdated
to handle changes in these properties and a helper methodisValidCSSValue
to validate the CSS values. The class is then declared globally. This seems to be a good approach to encapsulate the logic for handling padding and margin in a separate class. However, theupdated
method could be optimized to avoid unnecessary checks and operations.
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
Files selected for processing (1)
- .changeset/purple-crews-attack.md (1 hunks)
Additional comments (Suppressed): 1
.changeset/purple-crews-attack.md (1)
- 1-9: The changeset indicates that several packages have been patched, presumably related to the updates for core link best practices. Ensure that these patches have been applied correctly and that they don't introduce any breaking changes.
outline-core-link
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 (6)
- package.json
- packages/documentation/outline-docs/src/screenshots/outline-accordion--storybook.png
- packages/documentation/outline-docs/src/screenshots/outline-accordion--yarn-add.png
- packages/documentation/outline-docs/src/screenshots/outline-admin-links--storybook.png
- packages/documentation/outline-docs/src/screenshots/outline-admin-links--yarn-add.png
- yarn.lock
Files selected for processing (21)
- .storybook/main.js (2 hunks)
- .storybook/preview.js (1 hunks)
- packages/documentation/outline-docs/src/guides/development/component-development/00-main.mdx (1 hunks)
- packages/documentation/outline-docs/src/guides/development/component-development/01-component-structure.mdx (3 hunks)
- packages/documentation/outline-docs/src/guides/development/component-development/03-styles.mdx (2 hunks)
- packages/documentation/outline-docs/src/guides/development/component-development/04-controllers.mdx (2 hunks)
- packages/documentation/outline-docs/src/guides/development/component-development/05-render.mdx (2 hunks)
- packages/documentation/outline-docs/src/guides/development/component-development/06-properties.mdx (2 hunks)
- packages/documentation/outline-docs/src/guides/development/component-development/07-slots.mdx (2 hunks)
- packages/documentation/outline-docs/src/guides/development/component-development/08-lifecycle-methods.mdx (2 hunks)
- packages/documentation/outline-docs/src/guides/development/component-development/09-events.mdx (2 hunks)
- packages/documentation/outline-docs/src/guides/development/component-development/10-utility-functions.mdx (2 hunks)
- packages/documentation/outline-docs/src/guides/development/component-development/11-extending.mdx (2 hunks)
- packages/documentation/outline-docs/src/guides/development/tooling/01-tooling.mdx (1 hunks)
- packages/documentation/outline-docs/src/guides/development/tooling/02-helpful-commands.mdx (2 hunks)
- packages/documentation/outline-docs/src/guides/development/tooling/03-conventional-commits.mdx (2 hunks)
- packages/documentation/outline-docs/src/guides/development/tooling/04-production-builds.mdx (2 hunks)
- packages/documentation/outline-docs/src/guides/development/tooling/06-changesets.mdx (2 hunks)
- packages/documentation/outline-docs/src/guides/welcome/00-welcome.mdx (1 hunks)
- packages/documentation/outline-docs/src/guides/welcome/more-information/99-additional-resources.mdx (3 hunks)
- packages/documentation/outline-docs/src/guides/welcome/more-information/99-contributing.mdx (2 hunks)
Files not summarized due to errors (21)
- packages/documentation/outline-docs/src/guides/welcome/00-welcome.mdx: Error: Server error. Please try again later.
- packages/documentation/outline-docs/src/guides/development/tooling/03-conventional-commits.mdx: Error: Server error. Please try again later.
- packages/documentation/outline-docs/src/guides/development/component-development/07-slots.mdx: Error: Server error. Please try again later.
- packages/documentation/outline-docs/src/guides/development/component-development/00-main.mdx: Error: Server error. Please try again later.
- packages/documentation/outline-docs/src/guides/development/tooling/06-changesets.mdx: Error: Server error. Please try again later.
- packages/documentation/outline-docs/src/guides/development/component-development/06-properties.mdx: Error: Server error. Please try again later.
- packages/documentation/outline-docs/src/guides/development/component-development/01-component-structure.mdx: Error: Server error. Please try again later.
- packages/documentation/outline-docs/src/guides/development/component-development/04-controllers.mdx: Error: Server error. Please try again later.
- packages/documentation/outline-docs/src/guides/development/component-development/11-extending.mdx: Error: Server error. Please try again later.
- packages/documentation/outline-docs/src/guides/welcome/more-information/99-additional-resources.mdx: Error: Server error. Please try again later.
- packages/documentation/outline-docs/src/guides/welcome/more-information/99-contributing.mdx: Error: Server error. Please try again later.
- packages/documentation/outline-docs/src/guides/development/component-development/09-events.mdx: Error: Server error. Please try again later.
- packages/documentation/outline-docs/src/guides/development/component-development/10-utility-functions.mdx: Error: Server error. Please try again later.
- .storybook/preview.js: Error: Server error. Please try again later.
- .storybook/main.js: Error: Server error. Please try again later.
- packages/documentation/outline-docs/src/guides/development/component-development/08-lifecycle-methods.mdx: Error: Server error. Please try again later.
- packages/documentation/outline-docs/src/guides/development/component-development/03-styles.mdx: Error: Server error. Please try again later.
- packages/documentation/outline-docs/src/guides/development/component-development/05-render.mdx: Error: Server error. Please try again later.
- packages/documentation/outline-docs/src/guides/development/tooling/02-helpful-commands.mdx: Error: Server error. Please try again later.
- packages/documentation/outline-docs/src/guides/development/tooling/01-tooling.mdx: Error: Server error. Please try again later.
- packages/documentation/outline-docs/src/guides/development/tooling/04-production-builds.mdx: Error: Server error. Please try again later.
Files not reviewed due to errors (21)
- packages/documentation/outline-docs/src/guides/development/tooling/04-production-builds.mdx (Error: Server error. Please try again later.)
- packages/documentation/outline-docs/src/guides/development/component-development/07-slots.mdx (Error: Server error. Please try again later.)
- packages/documentation/outline-docs/src/guides/development/component-development/04-controllers.mdx (Error: Server error. Please try again later.)
- packages/documentation/outline-docs/src/guides/development/component-development/08-lifecycle-methods.mdx (Error: Server error. Please try again later.)
- packages/documentation/outline-docs/src/guides/development/component-development/09-events.mdx (Error: Server error. Please try again later.)
- packages/documentation/outline-docs/src/guides/welcome/more-information/99-contributing.mdx (Error: Server error. Please try again later.)
- .storybook/preview.js (Error: Server error. Please try again later.)
- .storybook/main.js (Error: Server error. Please try again later.)
- packages/documentation/outline-docs/src/guides/development/component-development/11-extending.mdx (Error: Server error. Please try again later.)
- packages/documentation/outline-docs/src/guides/development/component-development/00-main.mdx (Error: Server error. Please try again later.)
- packages/documentation/outline-docs/src/guides/development/component-development/01-component-structure.mdx (Error: Server error. Please try again later.)
- packages/documentation/outline-docs/src/guides/development/component-development/10-utility-functions.mdx (Error: Server error. Please try again later.)
- packages/documentation/outline-docs/src/guides/development/component-development/03-styles.mdx (Error: Server error. Please try again later.)
- packages/documentation/outline-docs/src/guides/welcome/more-information/99-additional-resources.mdx (Error: Server error. Please try again later.)
- packages/documentation/outline-docs/src/guides/welcome/00-welcome.mdx (Error: Server error. Please try again later.)
- packages/documentation/outline-docs/src/guides/development/tooling/01-tooling.mdx (Error: Server error. Please try again later.)
- packages/documentation/outline-docs/src/guides/development/component-development/05-render.mdx (Error: Server error. Please try again later.)
- packages/documentation/outline-docs/src/guides/development/tooling/02-helpful-commands.mdx (Error: Server error. Please try again later.)
- packages/documentation/outline-docs/src/guides/development/component-development/06-properties.mdx (Error: Server error. Please try again later.)
- packages/documentation/outline-docs/src/guides/development/tooling/06-changesets.mdx (Error: Server error. Please try again later.)
- packages/documentation/outline-docs/src/guides/development/tooling/03-conventional-commits.mdx (Error: Server error. Please try again later.)
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 (1)
- packages/documentation/outline-storybook/stories/components/outline-dropdown.stories.ts (2 hunks)
Files not summarized due to errors (1)
- packages/documentation/outline-storybook/stories/components/outline-dropdown.stories.ts: Error: Server error. Please try again later.
Files not reviewed due to errors (1)
- packages/documentation/outline-storybook/stories/components/outline-dropdown.stories.ts (Error: Server error. Please try again later.)
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: 4
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- package.json
- yarn.lock
Files selected for processing (3)
- .nvmrc (1 hunks)
- packages/documentation/outline-docs/index.ts (1 hunks)
- packages/documentation/outline-docs/src/guides/welcome/00-welcome.mdx (1 hunks)
Additional comments: 1
packages/documentation/outline-docs/index.ts (1)
- 16-23: The export
OutlineDocsStorybookDefaultParams
is commented out. If this is intentional to provide a template or example for future use, it's fine. Otherwise, if it's meant to be used, the code should be uncommented.
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 path filters (6)
package.json
is excluded by:!**/*.json
packages/components/outline-core-button/package.json
is excluded by:!**/*.json
packages/components/outline-core-button/tsconfig.build.json
is excluded by:!**/*.json
packages/controllers/adopted-stylesheets/package.json
is excluded by:!**/*.json
packages/documentation/outline-storybook/package.json
is excluded by:!**/*.json
yarn.lock
is excluded by:!**/*.lock
Files selected for processing (17)
- .storybook/storybook.css (1 hunks)
- packages/components/outline-core-button/CHANGELOG.md (1 hunks)
- packages/components/outline-core-button/docs/outline-core-button.mdx (1 hunks)
- packages/components/outline-core-button/index.ts (1 hunks)
- packages/components/outline-core-button/src/css-variables/vars-default.css (1 hunks)
- packages/components/outline-core-button/src/css-variables/vars-link.css (1 hunks)
- packages/components/outline-core-button/src/css-variables/vars-primary.css (1 hunks)
- packages/components/outline-core-button/src/css-variables/vars-secondary.css (1 hunks)
- packages/components/outline-core-button/src/css-variables/vars-tertiary.css (1 hunks)
- packages/components/outline-core-button/src/outline-core-button.css (1 hunks)
- packages/components/outline-core-button/src/outline-core-button.lightdom.css (1 hunks)
- packages/components/outline-core-button/src/outline-core-button.ts (1 hunks)
- packages/components/outline-core-link/src/outline-core-link.ts (6 hunks)
- packages/controllers/adopted-stylesheets/CHANGELOG.md (1 hunks)
- packages/controllers/adopted-stylesheets/README.md (1 hunks)
- packages/documentation/outline-storybook/CHANGELOG.md (1 hunks)
- packages/documentation/outline-storybook/stories/components/outline-core-button.stories.mdx (1 hunks)
Files skipped from review due to trivial changes (3)
- .storybook/storybook.css
- packages/controllers/adopted-stylesheets/CHANGELOG.md
- packages/documentation/outline-storybook/CHANGELOG.md
Additional comments: 41
packages/components/outline-core-button/index.ts (1)
- 1-2: The export statements for
OutlineCoreButton
and its associated types are clear and concise.packages/components/outline-core-button/CHANGELOG.md (1)
- 3-7: The changelog correctly documents the patch changes for version
0.0.2
.packages/components/outline-core-button/src/css-variables/vars-default.css (1)
- 18-22: The CSS custom properties are correctly using the
var()
function to provide a fallback value.packages/controllers/adopted-stylesheets/README.md (1)
- 37-37: The import statement in the example code correctly reflects the updated class name
AdoptedStyleSheets
.packages/components/outline-core-button/src/css-variables/vars-secondary.css (1)
- 39-73: The CSS custom properties for the secondary variant of the
outline-core-button
are well-defined and follow the established pattern of using computed variables with fallbacks.packages/components/outline-core-button/src/css-variables/vars-tertiary.css (1)
- 38-72: The CSS custom properties for the tertiary variant of the
outline-core-button
are correctly set up with computed values and appropriate fallbacks.packages/components/outline-core-button/src/css-variables/vars-link.css (1)
- 44-83: The CSS custom properties for the link variant of the
outline-core-button
are correctly using computed values and fallbacks.packages/components/outline-core-button/src/css-variables/vars-primary.css (1)
- 39-73: The CSS custom properties for the primary variant of the
outline-core-button
are correctly using computed values and fallbacks.packages/components/outline-core-button/src/outline-core-button.ts (6)
- 36-37: The
static styles
property is correctly assigned the component styles.- 50-56: The
connectedCallback
method is correctly setting up the adopted stylesheets.- 82-89: The
buttonVariant
property converter function ensures that only valid variants are set, defaulting to 'primary' if an invalid variant is provided.- 97-106: The
buttonSize
property converter function ensures that only valid sizes are set, defaulting to the first size type if an invalid size is provided.- 123-147: The
render
method correctly applies classes to slotted button elements based on the component's properties.- 150-156: The
updated
lifecycle method is correctly managing thearia-disabled
attribute based on theis-disabled
property.packages/components/outline-core-link/src/outline-core-link.ts (5)
- 1-1: The import statement correctly includes
css
from 'lit', which is necessary for the adopted stylesheets.- 8-9: The import paths for component styles and global styles have been updated to reflect the new directory structure.
- 74-80: The
connectedCallback
method is correctly setting up the adopted stylesheets.- 151-156: The
hasSlottedLink
method correctly checks for ana
tag within the slotted content.- 200-202: The
fullMarkupInSlot
method correctly adjusts slotted content and provides debugging if necessary.packages/components/outline-core-button/src/outline-core-button.lightdom.css (11)
- 1-9: The CSS for the button element is correctly resetting all properties to their initial values.
- 11-26: The
.is-disabled
class is correctly styled to prevent user interaction and is using computed variables for colors.- 28-45: The button styles are correctly setting display, alignment, cursor, and transition properties.
- 47-51: The
.is-disabled
class is correctly styled to prevent user interaction.- 53-79: The
.primary
class is correctly styled with computed variables for various states.- 81-100: The
.primary.is-disabled
class is correctly styled for the disabled state with computed variables.- 103-129: The
.secondary
class is correctly styled with computed variables for various states.- 131-150: The
.secondary.is-disabled
class is correctly styled for the disabled state with computed variables.- 153-179: The
.tertiary
class is correctly styled with computed variables for various states.- 181-200: The
.tertiary.is-disabled
class is correctly styled for the disabled state with computed variables.- 205-218: The button sizing classes
.small
,.medium
, and.large
are correctly using CSS variables for font size and line height.packages/components/outline-core-button/src/outline-core-button.css (11)
- 1-5: Ensure that the nested imports are pointing to the correct paths after the reorganization of components.
Verification successful
The verification script has successfully located the CSS variable files within the
packages/components/outline-core-button/src/css-variables/
directory. This confirms that the nested import paths used in theoutline-core-button.css
file are correct and pointing to the existing files as per the repository's current state.* 36-55: The `button` element styles are well-defined. However, ensure that the `var` function is used consistently for all CSS properties that can be customized. * 57-61: The `[aria-disabled='true'] ::slotted(button)` selector is correctly styled for disabled state. Confirm that this is the intended behavior for slotted buttons and that it doesn't conflict with the styles defined for `.primary.is-disabled`, `.secondary.is-disabled`, and `.tertiary.is-disabled`.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the nested import paths are correct. fd 'vars-.*\.css' packages/components/outline-core-button/css-variables --exec cat {}Length of output: 230
Script:
#!/bin/bash # Search for the CSS variable files without assuming their location. fd 'vars-.*\.css'Length of output: 1296
Verification successful
The review comment is verified. The styles for
.primary.is-disabled
,.secondary.is-disabled
, and.tertiary.is-disabled
do not conflict with the[aria-disabled='true'] ::slotted(button)
selector. They all apply the samepointer-events: none;
rule, and the latter two also applycursor: not-allowed;
anduser-select: none;
, which are consistent with the disabled state styling for slotted buttons.* 63-88: The `.primary` class and its pseudo-classes `:hover` and `:focus` are using computed CSS variables. Verify that these variables are defined and that their values are computed as expected after the reorganization.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the styles for slotted buttons do not conflict with the disabled state styles. rg --type css "\[aria-disabled='true'\] ::slotted\(button\)" packages/components/outline-core-button/srcLength of output: 223
Script:
#!/bin/bash # Check for the presence of disabled state styles for primary, secondary, and tertiary button classes. rg --type css "\.primary\.is-disabled" packages/components/outline-core-button/src rg --type css "\.secondary\.is-disabled" packages/components/outline-core-button/src rg --type css "\.tertiary\.is-disabled" packages/components/outline-core-button/srcLength of output: 1135
Script:
#!/bin/bash # Extract the content of the disabled state styles for primary, secondary, and tertiary button classes. rg --type css -A 3 "\.primary\.is-disabled" packages/components/outline-core-button/src/outline-core-button.css rg --type css -A 3 "\.secondary\.is-disabled" packages/components/outline-core-button/src/outline-core-button.css rg --type css -A 3 "\.tertiary\.is-disabled" packages/components/outline-core-button/src/outline-core-button.cssLength of output: 625
Verification successful
The CSS variables for the
.primary
class and its pseudo-classes:hover
and:focus
are indeed defined in thevars-primary.css
file. The variables are set with default values and fallbacks, ensuring that they are computed as expected.* 91-110: The `.primary[aria-disabled='true']` selector and its `:focus` pseudo-class are using computed CSS variables for disabled state. Ensure that these variables are defined and that their values are computed correctly.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the CSS variables for the .primary class are defined and computed correctly. rg --type css "--outline-core-button--primary-.*--computed" packages/components/outline-core-button/css-variablesLength of output: 561
Script:
#!/bin/bash # Find files that likely contain CSS variable definitions. fd 'css-variables' --type f --extension css packages/components/outline-core-button # Assuming the above command finds a file named something like 'css-variables.css', we will then search for the CSS variables. # Note: The actual command to search within the files will be written after confirming the file names with the output of the `fd` command.Length of output: 83
Script:
#!/bin/bash # Broaden the search for any files that might contain 'css-variables' in their name within the outline-core-button component directory. fd 'css-variables' packages/components/outline-core-button # If no specific 'css-variables' file is found, search for the CSS variables across all CSS files in the outline-core-button component directory. fd --type f --extension css packages/components/outline-core-button --exec rg -- '--outline-core-button--primary-.*--computed'Length of output: 708
Script:
#!/bin/bash # Search for CSS files within the css-variables directory. fd --type f --extension css 'css-variables' packages/components/outline-core-button/src/ # Assuming the above command finds CSS files, we will then search for the CSS variables within those files. # Note: The actual command to search within the files will be written after confirming the file names with the output of the `fd` command.Length of output: 88
Script:
#!/bin/bash # Search for any CSS files within the css-variables directory. fd --type f --extension css packages/components/outline-core-button/src/css-variables/ # If CSS files are found within the css-variables directory, search for the CSS variables related to the .primary class. # Note: The actual command to search within the files will be written after confirming the file names with the output of the `fd` command.Length of output: 626
Script:
#!/bin/bash # Use the match-all pattern to search for CSS files within the css-variables directory. fd . 'packages/components/outline-core-button/src/css-variables/' --type f --extension css # If CSS files are found within the css-variables directory, search for the CSS variables related to the .primary class. # Note: The actual command to search within the files will be written after confirming the file names with the output of the `fd` command.Length of output: 465
Script:
#!/bin/bash # Search within vars-primary.css for the CSS variables related to the .primary class. rg -- '--outline-core-button--primary-.*--computed' packages/components/outline-core-button/src/css-variables/vars-primary.cssLength of output: 3422
Verification successful
The CSS variables for the
.primary[aria-disabled='true']
selector and its:focus
pseudo-class are indeed defined in thevars-primary.css
file within thecss-variables
directory of theoutline-core-button
component. The variables are set with default values, ensuring that they are computed correctly.* 113-138: The `.secondary` class and its pseudo-classes `:hover` and `:focus` are using computed CSS variables. Ensure that these variables are defined and that their values are computed correctly.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the CSS variables for the .primary[aria-disabled='true'] selector are defined and computed correctly. rg --type css "--outline-core-button--primary-disabled-.*--computed" packages/components/outline-core-button/css-variablesLength of output: 579
Script:
#!/bin/bash # Verify that the CSS variables for the .primary[aria-disabled='true'] selector are defined and computed correctly. rg --type css '\--outline-core-button--primary-disabled-.*--computed' packages/components/outline-core-button/css-variablesLength of output: 219
Script:
#!/bin/bash # Search for a file that likely contains the CSS variables definitions. fd 'css-variables.css' packages/components/outline-core-buttonLength of output: 62
Script:
#!/bin/bash # Broaden the search to look for any CSS files within the outline-core-button directory. fd '\.css$' packages/components/outline-core-buttonLength of output: 573
Script:
#!/bin/bash # Check the CSS variable definition files for the presence of the specific CSS variables mentioned in the review comment. rg --type css '\--outline-core-button--primary-disabled-.*--computed' $(fd '\.css$' packages/components/outline-core-button/src/css-variables)Length of output: 2433
Verification successful
The verification process has confirmed that the CSS variables for the
.secondary
class, along with its:hover
and:focus
pseudo-classes, are indeed defined and their values are computed correctly within thepackages/components/outline-core-button/src/css-variables/vars-secondary.css
file. The variables are set with default values and fallbacks, ensuring that the styles for the.secondary
class are properly computed.* 141-160: The `.secondary[aria-disabled='true']` selector and its `:focus` pseudo-class are using computed CSS variables for disabled state. Confirm that these variables are defined and that their values are computed correctly.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the CSS variables for the .secondary class are defined and computed correctly. rg --type css "--outline-core-button--secondary-.*--computed" packages/components/outline-core-button/css-variablesLength of output: 565
Script:
#!/bin/bash # Attempt to find the CSS variable definitions within CSS files in the correct directory. rg --type css --glob '*.css' "outline-core-button--secondary-.*--computed" packages/components/outline-core-button/src/Length of output: 11489
Verification successful
The CSS variables for the
.secondary[aria-disabled='true']
selector and its:focus
pseudo-class are indeed defined and their values are computed correctly. The variables are found in thevars-secondary.css
file within theoutline-core-button
component directory, as well as being referenced inoutline-core-button.lightdom.css
andoutline-core-button.css
.* 163-188: The `.tertiary` class and its pseudo-classes `:hover` and `:focus` are using computed CSS variables. Confirm that these variables are defined and that their values are computed correctly.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the CSS variables for the .secondary[aria-disabled='true'] selector are defined and computed correctly. rg --type css "--outline-core-button--secondary-disabled-.*--computed" packages/components/outline-core-button/css-variablesLength of output: 583
Script:
#!/bin/bash # Correcting the script to search for the CSS variables definitions in files that likely contain 'css-variables' in their name. rg --glob '*css-variables*' 'outline-core-button--secondary-disabled-.*--computed' packages/components/outline-core-button/Length of output: 125
Script:
#!/bin/bash # Searching for the CSS variable definitions within all CSS files in the `outline-core-button` directory. rg 'outline-core-button--secondary-disabled-.*--computed' packages/components/outline-core-button/ -g '*.css'Length of output: 4411
Verification successful
The verification script has successfully found the CSS variable definitions for the
.tertiary
class within the CSS files of theoutline-core-button
component. The variables are indeed defined and appear to be computed correctly, with fallbacks provided for each variable. This confirms that the CSS variables used in the.tertiary
class and its pseudo-classes:hover
and:focus
are properly established in the codebase.* 191-210: The `.tertiary[aria-disabled='true']` selector and its `:focus` pseudo-class are using computed CSS variables for disabled state. Ensure that these variables are defined and that their values are computed correctly.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the CSS variables for the .tertiary class are defined and computed correctly. rg --type css "--outline-core-button--tertiary-.*--computed" packages/components/outline-core-button/css-variablesLength of output: 563
Script:
#!/bin/bash # Adjusting the script to search for the CSS variables within CSS files of the outline-core-button component. # Using fd to find CSS files within the outline-core-button directory and then using rg to search for the variables. fd '.css$' packages/components/outline-core-button --exec rg -- "--outline-core-button--tertiary-.*--computed" {}Length of output: 6631
Verification successful
The CSS variables for the
.tertiary[aria-disabled='true']
selector and its:focus
pseudo-class are indeed defined in thevars-tertiary.css
file within thepackages/components/outline-core-button/src/css-variables/
directory. The variables are set with default values and fallbacks, ensuring that they are computed correctly.* 213-228: The `:host(.icon__button)` selector is applying `!important` to the border-radius properties. Confirm that this is necessary and that it doesn't override any other important styles unintentionally. * 230-244: The `.small`, `.medium`, and `.large` classes are using variables for font-size and line-height. Confirm that these variables are defined and that their values are set correctly.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the CSS variables for the .tertiary[aria-disabled='true'] selector are defined and computed correctly. rg --type css "--outline-core-button--tertiary-disabled-.*--computed" packages/components/outline-core-button/css-variablesLength of output: 581
Script:
#!/bin/bash # Verify that the CSS variables for the .tertiary[aria-disabled='true'] selector are defined and computed correctly. # Correcting the previous script by removing the double dashes before the pattern rg -e "--outline-core-button--tertiary-disabled-.*--computed" packages/components/outline-core-button/css-variablesLength of output: 211
Script:
#!/bin/bash # Attempting to find the correct location of the CSS variables file. fd "css-variables" packages/components/outline-core-buttonLength of output: 117
Script:
#!/bin/bash # Now that we have the correct directory, let's search for the CSS variables within it. rg -e "--outline-core-button--tertiary-disabled-.*--computed" packages/components/outline-core-button/src/css-variables/Length of output: 2060
Verification successful
The verification process has successfully identified the usage of the CSS variables
--fs-sm
,--fs-base
,--fs-lg
,--lh-sm
,--lh-base
, and--lh-lg
within the CSS files of theoutline-core-button
directory. These variables correspond to the font sizes and line heights for the.small
,.medium
, and.large
classes as mentioned in the review comment.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the variables for button sizing are defined and set correctly. rg --type css "--fs-.*" packages/components/outline-core-button/css-variables rg --type css "--lh-.*" packages/components/outline-core-button/css-variablesLength of output: 979
Script:
#!/bin/bash # Correcting the script to search for the CSS variables within the likely CSS files. # Using `fd` to find the relevant CSS file that would contain the variables. fd 'css-variables\.css$' packages/components/outline-core-button --exec rg --type css 'var\(--fs-.*\)' fd 'css-variables\.css$' packages/components/outline-core-button --exec rg --type css 'var\(--lh-.*\)'Length of output: 209
Script:
#!/bin/bash # Search for any CSS files within the outline-core-button directory. fd '\.css$' packages/components/outline-core-button --exec rg --type css 'var\(--fs-.*\)' fd '\.css$' packages/components/outline-core-button --exec rg --type css 'var\(--lh-.*\)'Length of output: 527
packages/documentation/outline-storybook/stories/components/outline-core-button.stories.mdx
Show resolved
Hide resolved
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.
This is awesome, thank you!
TODO:
|
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 path filters (2)
package.json
is excluded by:!**/*.json
yarn.lock
is excluded by:!**/*.lock
Files selected for processing (1)
- .yarnrc.yml (1 hunks)
Additional comments: 2
.yarnrc.yml (2)
- 1-1: Ensure the
nodeLinker: node-modules
setting aligns with the project's dependency management strategy, especially if transitioning from Yarn 2's Plug'n'Play to a traditionalnode_modules
layout.- 3-3: Verify the specified
yarnPath
points to the correct Yarn release version and file format for the project's needs. Consider potential impacts on CI/CD pipelines and local development environments.
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 (1)
- .changeset/shaggy-ducks-cheat.md (1 hunks)
Files not reviewed due to errors (1)
- .changeset/shaggy-ducks-cheat.md (Error: unable to parse review)
Summary by CodeRabbit