Skip to content

feat(client): improve styling of switch components in block settings#512

Merged
johbaxter merged 3 commits intodevfrom
styling-of-switch-components-486
Feb 20, 2025
Merged

feat(client): improve styling of switch components in block settings#512
johbaxter merged 3 commits intodevfrom
styling-of-switch-components-486

Conversation

@NayanKumar809
Copy link
Copy Markdown
Contributor

@NayanKumar809 NayanKumar809 commented Jan 30, 2025

After changes, the switch button looks like this:
image

Before changes, it was looking like this:
image

@NayanKumar809 NayanKumar809 requested a review from a team as a code owner January 30, 2025 07:03
@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /describe

@QodoAI-Agent
Copy link
Copy Markdown

Title

feat(client): improve styling of switch components in block settings


PR Type

Enhancement


Description

  • Improved styling for Switch component in block settings.

  • Added support for size prop in Switch component.

  • Enhanced SwitchSettings layout with tooltips and typography.

  • Introduced StyledTypography for better text wrapping in settings.


Changes walkthrough 📝

Relevant files
Enhancement
Switch.tsx
Add `size` prop and refine `Switch` styling                           

libs/ui/src/components/Switch/Switch.tsx

  • Removed hardcoded dimensions for StyledSwitch.
  • Added support for size prop in Switch component.
  • Simplified and improved styling logic for Switch.
  • +2/-8     
    SwitchSettings.tsx
    Improve `SwitchSettings` layout and add tooltips                 

    packages/client/src/components/block-settings/shared/SwitchSettings.tsx

  • Enhanced layout with Stack for better alignment.
  • Added tooltips with descriptions for settings.
  • Introduced StyledTypography for improved text wrapping.
  • Updated Switch to use size="small" for compact design.
  • +47/-9   

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /review

    @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /improve

    @QodoAI-Agent
    Copy link
    Copy Markdown

    QodoAI-Agent commented Jan 30, 2025

    PR Code Suggestions ✨

    Latest suggestions up to cbff03c

    CategorySuggestion                                                                                                                                    Impact
    Security
    Sanitize description to prevent XSS

    Ensure that the description prop is sanitized or validated to prevent potential
    security risks like XSS when rendering it inside the Tooltip component.

    packages/client/src/components/block-settings/shared/SwitchSettings.tsx [130]

    -<Tooltip placement="top" title={description} arrow>
    +<Tooltip placement="top" title={sanitize(description)} arrow>
    Suggestion importance[1-10]: 9

    __

    Why: Sanitizing the description prop is crucial for preventing XSS vulnerabilities, especially since it is rendered in a Tooltip. This is a high-impact security enhancement.

    High
    General
    Validate size prop for allowed values

    Ensure that the size prop passed to StyledSwitch is validated to only accept
    predefined values like "small", "medium", or "large" to avoid unexpected behavior or
    styling issues.

    libs/ui/src/components/Switch/Switch.tsx [120-121]

     const { sx, size = "medium" } = props;
    -return <StyledSwitch sx={sx} size={size} {...props} />;
    +const validSizes = ["small", "medium", "large"];
    +const validatedSize = validSizes.includes(size) ? size : "medium";
    +return <StyledSwitch sx={sx} size={validatedSize} {...props} />;
    Suggestion importance[1-10]: 8

    __

    Why: Validating the size prop ensures that only predefined values are used, preventing unexpected behavior or styling issues. This is a significant improvement in terms of robustness and maintainability.

    Medium
    Possible issue
    Add error handling for onChange

    Add error handling for the onChange function to ensure it gracefully handles
    potential issues, such as when onChange is undefined or throws an error.

    packages/client/src/components/block-settings/shared/SwitchSettings.tsx [151]

    -onChange(!value);
    +if (onChange) {
    +    try {
    +        onChange(!value);
    +    } catch (error) {
    +        console.error("Error in onChange handler:", error);
    +    }
    +}
    Suggestion importance[1-10]: 7

    __

    Why: Adding error handling for the onChange function enhances the code's resilience by preventing runtime errors if onChange is undefined or fails. This is a practical improvement for better error management.

    Medium

    Previous suggestions

    Suggestions up to commit cbff03c
    CategorySuggestion                                                                                                                                    Impact
    Security
    Sanitize description to prevent injection

    Ensure the description prop is sanitized or validated to prevent potential security
    risks such as injection attacks when rendering user-provided content in the tooltip.

    packages/client/src/components/block-settings/shared/SwitchSettings.tsx [130]

    -<Tooltip placement="top" title={description} arrow>
    +<Tooltip placement="top" title={sanitize(description)} arrow>
    Suggestion importance[1-10]: 9

    __

    Why: Sanitizing the description prop is crucial for preventing potential security risks such as injection attacks. This is a high-impact improvement for ensuring the security of the application.

    High
    General
    Validate size prop to prevent misuse

    Ensure that the size prop is validated to only accept predefined values such as
    "small", "medium", or "large" to prevent unexpected behavior or styling issues.

    libs/ui/src/components/Switch/Switch.tsx [4-6]

    -const StyledSwitch = styled(MuiSwitch)(({ theme, size }) => ({
    -    width: size === "small" ? "40px" : "52px",
    -    height: size === "small" ? "20px" : "32px",
    +const StyledSwitch = styled(MuiSwitch)(({ theme, size }) => {
    +    if (!["small", "medium", "large"].includes(size)) {
    +        console.warn(`Invalid size prop: ${size}`);
    +        size = "medium"; // default fallback
    +    }
    +    return {
    +        width: size === "small" ? "40px" : "52px",
    +        height: size === "small" ? "20px" : "32px",
    +    };
    +});
    Suggestion importance[1-10]: 8

    __

    Why: Adding validation for the size prop ensures that only predefined values are used, preventing unexpected behavior or styling issues. This is a significant improvement for robustness and maintainability.

    Medium
    Validate value prop for Switch

    Ensure that the value prop passed to the Switch component is properly validated to
    avoid runtime errors or unexpected behavior.

    packages/client/src/components/block-settings/shared/SwitchSettings.tsx [147-153]

     <Switch
    -    checked={value}
    +    checked={!!value}
         onChange={() => {
             // sync the data on change
             onChange(!value);
         }}
         size="small"
     />
    Suggestion importance[1-10]: 6

    __

    Why: Validating the value prop ensures that it is properly handled as a boolean, reducing the risk of runtime errors or unexpected behavior. While this is a useful enhancement, its impact is moderate compared to other suggestions.

    Low
    Possible issue
    Add error handling for onChange

    Add error handling for the onChange function to ensure that any issues during state
    synchronization are logged or managed gracefully.

    packages/client/src/components/block-settings/shared/SwitchSettings.tsx [151]

    -onChange(!value);
    +try {
    +    onChange(!value);
    +} catch (error) {
    +    console.error("Error updating switch value:", error);
    +}
    Suggestion importance[1-10]: 7

    __

    Why: Adding error handling for the onChange function improves the resilience of the code by ensuring that any issues during state synchronization are logged or managed gracefully. This is a practical enhancement for debugging and reliability.

    Medium
    Suggestions up to commit 45d8c62
    CategorySuggestion                                                                                                                                    Score
    Security
    Sanitize description prop for safety

    Ensure that the description prop is sanitized or validated to prevent potential
    security risks like injection attacks when rendering user-provided content in the
    tooltip.

    packages/client/src/components/block-settings/shared/SwitchSettings.tsx [130]

    -<Tooltip placement="top" title={description} arrow>
    +<Tooltip placement="top" title={sanitize(description)} arrow>
    Suggestion importance[1-10]: 9

    Why: Sanitizing the description prop is crucial for preventing potential security vulnerabilities like injection attacks, especially when rendering user-provided content. This is a significant improvement for application security.

    9
    General
    Add default value for size prop

    Ensure that the size prop passed to the StyledSwitch component is validated or has a
    default value to prevent potential runtime errors if size is undefined or invalid.

    libs/ui/src/components/Switch/Switch.tsx [115-116]

    -const { sx, size } = props;
    +const { sx, size = "medium" } = props;
     return <StyledSwitch sx={sx} size={size} {...props} />;
    Suggestion importance[1-10]: 8

    Why: Adding a default value for the size prop ensures that the StyledSwitch component behaves predictably even if the size prop is not provided. This prevents potential runtime errors and improves code robustness.

    8
    Add fallback for label prop

    Add a fallback or default value for the label prop to ensure proper rendering when
    label is undefined or empty.

    packages/client/src/components/block-settings/shared/SwitchSettings.tsx [126-128]

     <StyledTypography variant="body2">
    -    {label}
    +    {label || "Default Label"}
     </StyledTypography>
    Suggestion importance[1-10]: 6

    Why: Adding a fallback for the label prop ensures proper rendering even when the label is undefined or empty. This improves the user experience and prevents potential UI issues.

    6
    Possible issue
    Add error handling for onChange

    Add error handling for the onChange function to ensure that unexpected errors during
    state updates do not cause application crashes.

    packages/client/src/components/block-settings/shared/SwitchSettings.tsx [144]

    -onChange(!value);
    +try {
    +    onChange(!value);
    +} catch (error) {
    +    console.error("Error updating switch state:", error);
    +}
    Suggestion importance[1-10]: 7

    Why: Adding error handling for the onChange function enhances the application's resilience by preventing crashes due to unexpected errors during state updates. This is a valuable improvement for robustness.

    7

    @neelneelneel
    Copy link
    Copy Markdown
    Contributor

    @natdink Thoughts? This doesn't match the spec

    @natdink
    Copy link
    Copy Markdown
    Contributor

    natdink commented Feb 6, 2025

    @natdink Thoughts? This doesn't match the spec

    Agree, not sure what the original request was but as a team we had decided to use the "apple" styling of the switch component. I think if anything we need to work on the styling of the existing switch component to provide sizing options and scale appropriately.

    I know we aren't on Material 3 yet but you should be able to utilize their specs on the switch component: https://m3.material.io/components/switch/overview

    @anurag91jain anurag91jain linked an issue Feb 6, 2025 that may be closed by this pull request
    3 tasks
    @johbaxter
    Copy link
    Copy Markdown
    Contributor

    Im giving this a pass.

    Nayan added custom styles on the size property for switch, nothing bad pushed in just a smaller version of what we already have that represents an apple switch.

    I'm not seeing anything the material 3 link from above specifying multiple sizes of the switch, no visual indicators. But as for the normal size of switch that has not been affected.

    If this is a big issue on the styling of the switch we can bring this up in a new ticket. Looking at styles im good with it, if you see anything please point out the exact pixel we are messing up

    @johbaxter johbaxter merged commit 36173b8 into dev Feb 20, 2025
    @johbaxter johbaxter deleted the styling-of-switch-components-486 branch February 20, 2025 21:45
    @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /update_changelog

    @QodoAI-Agent
    Copy link
    Copy Markdown

    Changelog updates: 🔄

    2025-02-20

    Changed

    • Improved styling and layout of switch components in block settings for better usability and appearance.

    to commit the new content to the CHANGELOG.md file, please type:
    '/update_changelog --pr_update_changelog.push_changelog_changes=true'

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    None yet

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    Improve styling of switch components in Block Settings

    5 participants