-
Notifications
You must be signed in to change notification settings - Fork 12
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
Release 9.5.0 #803
Release 9.5.0 #803
Conversation
WalkthroughThe latest release brings a host of improvements and fixes. Highlights include new utility classes for dimensions, enhanced 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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #803 +/- ##
========================================
Coverage 25.04% 25.04%
========================================
Files 36 36
Lines 1206 1206
Branches 305 305
========================================
Hits 302 302
Misses 631 631
Partials 273 273
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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 path filters (1)
package.json
is excluded by:!**/*.json
Files selected for processing (15)
- CHANGELOG.md (1 hunks)
- scripts/components/collapsable/collapsable.js (3 hunks)
- scripts/components/color-picker-component/color-picker-component.js (6 hunks)
- scripts/components/index.js (1 hunks)
- scripts/components/number-picker/number-picker.js (6 hunks)
- scripts/components/number-picker/number-picker.scss (1 hunks)
- scripts/components/responsive-number-picker/responsive-number-picker.js (2 hunks)
- scripts/components/toggle/docs/readme.mdx (1 hunks)
- scripts/components/toggle/docs/story.js (8 hunks)
- scripts/components/toggle/toggle.js (3 hunks)
- scripts/editor/icons/icons.js (3 hunks)
- scripts/index.js (1 hunks)
- styles/es-component-styles.scss (1 hunks)
- styles/es-gutenberg-component-style-updates.scss (1 hunks)
- styles/es-utility-classes.scss (22 hunks)
Files skipped from review due to trivial changes (1)
- styles/es-component-styles.scss
Additional comments: 39
scripts/components/toggle/docs/readme.mdx (1)
- 6-13: The documentation update correctly reflects the renaming of the
IconToggle
component toToggle
and provides a clear, concise example of its usage. This change aligns with the PR objectives and enhances the clarity of the component's documentation.scripts/components/number-picker/number-picker.scss (1)
- 1-32: The CSS changes introduced in
number-picker.scss
enhance the visual appearance and usability of theNumberPicker
component. The addition of new styles and adjustments to existing ones are well-structured and follow CSS best practices. However, it's recommended to verify the removal of styles for.components-button > svg
to ensure it doesn't unintentionally impact other components or parts of the UI.scripts/components/toggle/docs/story.js (1)
- 17-23: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [8-162]
The updates in
story.js
accurately reflect the renaming of theIconToggle
component toToggle
and provide a comprehensive set of examples demonstrating the component's versatility and usage in various scenarios. This enhances the documentation and assists developers in understanding how to implement theToggle
component effectively.scripts/components/collapsable/collapsable.js (1)
- 32-46: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [19-70]
The addition of the
disabled
prop to theCollapsable
component, along with the logic to automatically close the component when it is disabled and open, and the dynamic label for the expand button, are thoughtful enhancements that improve the component's functionality and usability. These changes should be tested thoroughly to ensure smooth integration with existing implementations.scripts/components/toggle/toggle.js (1)
- 85-94: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [6-94]
The changes in
toggle.js
accurately reflect the renaming of theIconToggle
component toToggle
and handle the deprecation ofIconToggle
in a manner that ensures backward compatibility. This approach follows best practices for component renaming and deprecation, providing a clear path for developers to transition to the new component name.scripts/components/index.js (1)
- 29-29: The update to the export statements in
index.js
correctly reflects the renaming of theIconToggle
component toToggle
and ensures backward compatibility by continuing to exportIconToggle
. This thoughtful approach facilitates a smooth transition for developers to the new component name without introducing breaking changes.styles/es-gutenberg-component-style-updates.scss (1)
- 78-78: The CSS change in
es-gutenberg-component-style-updates.scss
specifically targets.components-number-control
to refine its styling, excluding cases where it is nested within.es-number-picker .components-number-control
. This specificity is beneficial for improving the component's integration and appearance. However, it's recommended to test this change thoroughly to ensure it does not have unintended side effects on other components or parts of the UI.scripts/components/responsive-number-picker/responsive-number-picker.js (1)
- 120-144: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [126-162]
The refactoring of the
ResponsiveNumberPicker
component streamlines its structure and enhances maintainability and performance. The changes, including the optimized key assignment, conditional rendering of theextraButton
, and the application of classnames, are well-implemented. It's recommended to test these changes thoroughly to ensure smooth integration with existing implementations.scripts/components/number-picker/number-picker.js (5)
- 32-37: The addition of
prefix
,suffix
,prefixClass
,suffixClass
,extraButton
, andnoExtraButtonSeparator
props enhances the component's customization capabilities. Ensure that the documentation comments accurately describe the purpose and usage of these new props to maintain clarity and ease of use for developers.- 98-98: The dynamic style calculation for
--es-number-input-width
based onfixedWidth
andmin
props is a clever way to adjust the input field's width. However, ensure that this CSS variable is properly utilized in the component's stylesheet to affect the desired element.Verification successful
The CSS variable
--es-number-input-width
is indeed utilized in the component's stylesheet to dynamically adjust the width of an element, as confirmed by the script output. This verifies the effective use of the dynamic style calculation mentioned in the review comment.* 100-100: The conditional rendering of the `prefix` element with dynamic classes is well-implemented. It's important to ensure that the `prefixClass` or the default class provides the necessary styling without conflicts. * 119-119: Similar to the `prefix`, the conditional rendering and class assignment for the `suffix` element is handled well. Consistency in handling both prefix and suffix elements enhances the component's modularity and maintainability. * 152-160: The conditional rendering of the `extraButton` and its separator based on `noExtraButtonSeparator` is a good example of providing customizable UI components. Ensure that the `extraButton` prop is documented to accept any React component for maximum flexibility.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the CSS variable --es-number-input-width is utilized in the component's stylesheet fd 'number-picker\.scss' --exec grep -- '--es-number-input-width'Length of output: 116
scripts/index.js (1)
- 30-30: The update to export both
Toggle
andIconToggle
from thetoggle
component is correctly implemented. This change ensures backward compatibility while transitioning to the newToggle
naming convention. It's important to communicate this change clearly in the documentation to avoid confusion among developers.scripts/components/color-picker-component/color-picker-component.js (2)
- 40-40: The addition of the
additionalControls
prop to theColorPicker
component is a valuable enhancement, allowing for greater customization by enabling the inclusion of custom controls at the bottom of the popover. Ensure that this prop is documented clearly, including the expected type (React component) and usage examples if possible.- 227-231: The conditional rendering of
additionalControls
within theColorPicker
component is correctly implemented. This approach maintains the component's flexibility without introducing unnecessary complexity. It's a good practice to ensure that any passed controls are properly styled to match the component's design.CHANGELOG.md (1)
- 7-26: The changelog for version 9.5.0 is well-documented and follows the conventional format. It provides a clear and detailed list of additions, updates, and fixes, ensuring users and developers are well-informed about the changes in this release.
styles/es-utility-classes.scss (19)
- 323-323: The update from
border-top
toborder-block-start
is correctly applied, aligning with modern CSS practices for logical properties. This change enhances layout flexibility and supports different writing modes, such as right-to-left languages.- 332-332: The change from
border-bottom
toborder-block-end
is appropriate and follows the same rationale as the previous comment, promoting better internationalization and responsive design.- 341-341: Switching from
border-left
toborder-inline-start
is correctly implemented. This logical property is particularly useful for supporting layouts that need to adapt to both left-to-right and right-to-left reading directions.- 350-350: The update from
border-right
toborder-inline-end
is consistent with the shift towards logical properties, ensuring that the styling adapts based on the direction of the content.- 395-395: The transition from
margin-top
tomargin-block-start
is correctly applied. This change is part of the broader move to logical properties, improving the adaptability of margins in different writing modes.- 404-404: Updating
margin-bottom
tomargin-block-end
is correctly done, aligning with the logical properties approach for better responsiveness and internationalization support.- 413-413: The change from
margin-left
tomargin-inline-start
is appropriate, enhancing support for various writing modes by using logical properties.- 422-422: Switching from
margin-right
tomargin-inline-end
is correctly implemented, following the logical properties strategy for more adaptable and responsive design.- 458-458: The update from
padding-top
topadding-block-start
is correctly applied, aligning with the use of logical properties for better layout flexibility across different writing modes.- 467-467: Changing
padding-bottom
topadding-block-end
is correctly done, supporting the shift towards logical properties for improved responsiveness and internationalization.- 476-476: The transition from
padding-left
topadding-inline-start
is appropriate, enhancing the adaptability of padding in layouts that need to support both LTR and RTL directions.- 485-485: Updating
padding-right
topadding-inline-end
is consistent with the move to logical properties, ensuring that padding behaves correctly in different content flow directions.- 503-503: The change from
width
toinline-size
is correctly implemented. This logical property allows for more flexible layout management, especially in contexts where the writing mode may change.- 512-512: Switching from
height
toblock-size
is appropriate and aligns with the use of logical properties for better adaptability in various writing modes and directions.- 629-629: The update from
border-top-left-radius
toborder-start-start-radius
is correctly applied, aligning with the logical properties approach for more adaptable and responsive border-radius styling.- 638-638: Changing
border-top-right-radius
toborder-start-end-radius
is correctly done, supporting the shift towards logical properties for improved border-radius management in different writing modes.- 647-647: The transition from
border-bottom-left-radius
toborder-end-start-radius
is appropriate, enhancing the adaptability of border-radius in layouts that need to support both LTR and RTL directions.- 656-656: Updating
border-bottom-right-radius
toborder-end-end-radius
is consistent with the move to logical properties, ensuring that border-radius behaves correctly in different content flow directions.- 1249-1311: The introduction of new utility classes for setting dimensions (
es-size
,es-min-size
,es-max-size
, and their-nested
variants) is a significant enhancement. These classes provide a more concise and flexible way to apply sizing styles, including the special handling for SVG elements with the-nested
variants. This addition aligns with the PR's objectives to introduce enhancements and updates across the software's components and utilities.scripts/editor/icons/icons.js (3)
- 372-372: The addition of
strokeWidth
,fill
,fillOpacity
, and other attributes to thetoggleOff
icon enhances its styling flexibility. However, ensure that the use of CSS variables (e.g.,var(--es-toggle-icon-container-stroke, currentColor)
) is consistent across your project and that these variables are defined somewhere accessible to this icon. Also, consider accessibility implications, such as ensuring sufficient contrast and visibility for users with visual impairments.- 727-728: The
extract
andextractAlt
icons have been updated with new SVG paths. Ensure that these changes accurately represent the intended visual changes and that the icons remain clear and understandable at the sizes they will be used. Additionally, verify that these icons are used consistently across the application to represent the actions or concepts they are intended for, avoiding confusion with similar icons.- 748-748: The
hoverBackgroundGlow
icon introduces a complex SVG with a filter effect for a glow appearance. While visually appealing, consider the performance implications of SVG filters, especially on low-powered devices or in scenarios where many such icons might be rendered simultaneously. Testing across different devices and browsers is recommended to ensure a smooth user experience. Additionally, ensure that the filter id (#796755cf-6087-4f90-9a30-7138bc95e504
) is unique to avoid conflicts with other SVG elements that might use filters.
} = props; | ||
|
||
const spinnerButtonClass = 'es-w-4! es-h-3! es-min-w-4! es-rounded-1! es-button-icon-12 es-p-0! es-hover-bg-cool-gray-100! es-transition'; | ||
const prefixSuffixDefaultClass = 'es-user-select-none es-color-cool-gray-450'; |
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.
Tip
Codebase Verification
The executed script aimed to verify the presence of specific CSS classes within the number-picker.scss
file by searching for each class individually. The output clearly indicates that none of the specified classes were found in the number-picker.scss
file. This result suggests that the CSS classes defined in the spinnerButtonClass
variable are not included in the component's stylesheet as expected. Therefore, this could potentially lead to styling issues if these classes are essential for the component's appearance and functionality but are not defined elsewhere in the project's stylesheets.
Given the evidence from the script's output, it is appropriate to conclude that the review comment is verified, indicating a potential issue with the styling consistency and centralization for the spinner buttons of the number-picker
component.
Analysis chain
The spinnerButtonClass
variable defines a set of CSS classes for the spinner buttons. It's good practice to keep styling consistent and centralized. However, ensure that these classes are defined in a stylesheet that is loaded with this component to avoid styling issues.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that the defined CSS classes for spinnerButtonClass are included in the component's stylesheet
fd 'number-picker\.scss' --exec grep -- 'es-w-4! es-h-3! es-min-w-4! es-rounded-1! es-button-icon-12 es-p-0! es-hover-bg-cool-gray-100! es-transition'
Length of output: 152
Script:
#!/bin/bash
# Break down the search to look for key individual classes within the number-picker.scss file
classes=("es-w-4!" "es-h-3!" "es-min-w-4!" "es-rounded-1!" "es-button-icon-12" "es-p-0!" "es-hover-bg-cool-gray-100!" "es-transition")
file=$(fd 'number-picker\.scss')
for class in "${classes[@]}"; do
grep -q "$class" "$file" && echo "Found $class in $file" || echo "Did not find $class in $file"
done
Length of output: 2466
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)
- scripts/components/number-picker/number-picker.scss (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- scripts/components/number-picker/number-picker.scss
Added
es-size
,es-min-size
, andes-max-size
(+-nested
variants) utility classes for setting height and width at onceprefix
,suffix
,extraButton
, andnoExtraButtonSeparator
props inNumberPicker
Collapsable
now has adisabled
prop to disable the expand/collapse buttonUpdated
NumberPicker
has been completely updated visuallyResponsiveNumberPicker
now better visually integrates the reset buttonLinkInput
should now pop the suggestion panel only when focused into the input fieldMultiSelect
storyNumberControl
so they don't override FE libsNumberPicker
IconToggle
is nowToggle
, but the previous name will also work (for now)ColorPicker
now has a slot for additional controlsSummary by CodeRabbit
New Features
NumberPicker
for enhanced customization.disabled
property for theCollapsable
component to control its behavior.ColorPicker
component.Toggle
andIconToggle
components.NumberPicker
andResponsiveNumberPicker
components for a better user experience.Bug Fixes
Refactor
ResponsiveNumberPicker
component for better performance and maintainability.IconToggle
toToggle
to better reflect its functionality.Style
Documentation
IconToggle
toToggle
and to showcase new component features.Chores