Skip to content

Conversation

@IshanPhadteSolace
Copy link
Contributor

@IshanPhadteSolace IshanPhadteSolace commented Jun 30, 2025

PR Type

Enhancement, Documentation


Description

  • Refactored the SolaceRadio component by removing custom icons and simplifying the component structure.
  • Enhanced radio button styling in the theme file, providing more detailed and accessible styles for various states.
  • Implemented a new GitHub Actions workflow for generating and publishing MRC Usage Reports:
    • Generates reports for both maas-ui and maas-ops-ui repositories
    • Merges the reports and splits the JSON for easier consumption
    • Deploys the merged HTML report to GitHub Pages for easy access
  • Improved code maintainability and alignment with MUI standards in the radio component.
  • Enhanced documentation and automation for tracking component usage across projects.

Changes walkthrough 📝

Relevant files
Enhancement
SolaceRadio.tsx
Simplify SolaceRadio component                                                     

src/components/form/SolaceRadio.tsx

  • Removed import of RestingRadioIcon and SelectedRadioIcon
  • Removed icon and checkedIcon props from Radio component
  • +0/-3     
    theme.ts
    Refactor radio button styling in theme                                     

    src/resources/theme.ts

  • Replaced custom radio button styling with MUI-based styling
  • Added detailed styles for different states (enabled, hover, read-only,
    checked, disabled)
  • Improved accessibility with focus visible styles
  • +54/-29 
    Documentation
    mrc-usage-report.yml
    Add MRC Usage Report Generation workflow                                 

    .github/workflows/mrc-usage-report.yml

  • Added new workflow for generating MRC Usage Report
  • Includes steps to checkout repositories, set up Node.js, generate
    reports for maas-ui and maas-ops-ui
  • Merges reports, splits JSON, and deploys to GitHub Pages
  • +191/-0 

    @mre-leads mre-leads added documentation Improvements or additions to documentation enhancement New feature or request labels Jun 30, 2025
    @mre-leads
    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Security
    Reduce the scope of permissions for repository contents to improve security

    The 'contents: write' permission is overly broad for just checking out repositories.
    Consider using 'contents: read' instead, which is sufficient for checking out code.

    .github/workflows/mrc-usage-report.yml [14-17]

     permissions:
    -  contents: write # For checking out repositories
    +  contents: read # For checking out repositories
       pages: write # For publishing to GitHub Pages
       id-token: write # For OIDC authentication with GitHub Pages
     
    Suggestion importance[1-10]: 9

    Why: This suggestion addresses an important security concern by recommending the principle of least privilege. Using 'contents: read' instead of 'write' reduces potential security risks.

    9
    Best practice
    Avoid using !important by increasing CSS selector specificity

    The use of !important in the CSS should be avoided when possible. In this case, for the
    read-only checked state, consider restructuring the CSS selectors to increase specificity
    instead of using !important.

    src/resources/theme.ts [946-948]

    -"&.readOnly .MuiSvgIcon-root:last-of-type": {
    -  color: `${themeMapping.palette.secondary.wMain} !important`
    +"&.Mui-checked.readOnly .MuiSvgIcon-root:last-of-type": {
    +  color: themeMapping.palette.secondary.wMain
     }
     
    Suggestion importance[1-10]: 9

    Why: Removing !important and increasing selector specificity is a best practice that improves code quality and maintainability without significant risk.

    9
    Use a more appropriate branch name for the push trigger in the workflow

    Consider using a more specific branch name for the push trigger. The current branch name
    'iphadte/DATAGO-103052' appears to be a feature branch. It's generally a best practice to
    run scheduled workflows on stable branches like 'main' or 'master'.

    .github/workflows/mrc-usage-report.yml [3-9]

     on:
       schedule:
         - cron: "0 8 * * *" # Daily at 8 AM UTC
       workflow_dispatch: # Manual trigger
       push:
         branches:
    -      - iphadte/DATAGO-103052 # Trigger on pushes to the main branch
    +      - main # Trigger on pushes to the main branch
     
    Suggestion importance[1-10]: 8

    Why: The suggestion correctly identifies that using a feature branch for scheduled workflows is not a best practice. Using 'main' or a stable branch is more appropriate for this purpose.

    8
    Use a stable LTS version of Node.js instead of a non-LTS version

    The Node.js version specified (22) is currently not a stable LTS version. Consider using a
    stable LTS version like 20.x for better compatibility and long-term support.

    .github/workflows/mrc-usage-report.yml [72-75]

     - name: Set up Node.js
       uses: actions/setup-node@v4
       with:
    -    node-version: "22"
    +    node-version: "20"
     
    Suggestion importance[1-10]: 7

    Why: The suggestion correctly points out that using an LTS version of Node.js is better for stability and long-term support. However, the impact is moderate as the current version may still work.

    7
    Maintainability
    Extract radio button styles into a separate function for better organization

    Consider extracting the radio button styles into a separate function or object. This would
    improve code organization and make it easier to maintain and update the radio button
    styles independently of other theme components.

    src/resources/theme.ts [908-962]

    -"&.MuiRadio-root": {
    -  // ... all radio button styles ...
    +const getRadioStyles = (themeMapping) => ({
    +  "&.MuiRadio-root": {
    +    // ... all radio button styles ...
    +  }
    +});
    +
    +// In the main theme object
    +components: {
    +  MuiRadio: {
    +    styleOverrides: {
    +      root: getRadioStyles(themeMapping)
    +    }
    +  }
     }
     
    Suggestion importance[1-10]: 8

    Why: This suggestion would significantly improve code organization and maintainability without introducing major changes to the existing logic.

    8
    Use environment variables or inputs for repository names and branch references to improve workflow flexibility

    The workflow is using hardcoded paths and branch names, which may cause issues if the
    repository structure or branch names change. Consider using environment variables or
    workflow inputs for these values to make the workflow more flexible and maintainable.

    .github/workflows/mrc-usage-report.yml [32-38]

     - name: Checkout maas-ui
       uses: actions/checkout@v4
       with:
    -    repository: SolaceDev/maas-ui
    -    ref: sthomas/mrc-usage-report
    +    repository: ${{ env.MAAS_UI_REPO }}
    +    ref: ${{ env.MAAS_UI_BRANCH }}
         path: maas-ui-repo
         token: ${{ secrets.PACKAGES_ADMIN_TOKEN }}
     
    Suggestion importance[1-10]: 7

    Why: This suggestion improves the maintainability and flexibility of the workflow. Using variables instead of hardcoded values makes future updates easier and reduces potential errors.

    7
    Use CSS custom properties for colors to improve theme maintainability

    Consider using CSS custom properties (variables) for colors and other repeated values.
    This would make the theme more maintainable and easier to update in the future. You could
    define these variables at the root of your CSS and then reference them throughout the
    theme.

    src/resources/theme.ts [908-962]

    +":root": {
    +  "--color-secondary-w40": themeMapping.palette.secondary.w40,
    +  "--color-background-w10": themeMapping.palette.background.w10,
    +  "--color-secondary-wMain": themeMapping.palette.secondary.wMain,
    +  // ... more color variables ...
    +},
     "&.MuiRadio-root": {
    -  color: themeMapping.palette.secondary.w40,
    +  color: "var(--color-secondary-w40)",
       "& .MuiSvgIcon-root:first-of-type circle": {
    -    fill: themeMapping.palette.background.w10,
    +    fill: "var(--color-background-w10)",
         strokeWidth: "1px"
       },
       "&:hover": {
    -    color: themeMapping.palette.secondary.wMain
    +    color: "var(--color-secondary-wMain)"
       },
    -  // ... more styles ...
    +  // ... more styles using variables ...
     }
     
    Suggestion importance[1-10]: 7

    Why: Using CSS custom properties would indeed improve maintainability, but it's a significant change that might require adjustments in other parts of the codebase.

    7

    @sonarqube-solacecloud
    Copy link

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

    Labels

    documentation Improvements or additions to documentation enhancement New feature or request

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    3 participants