feat(button): enhance SVG handling and disabled state #1008

youngbeom-shin · 2025-03-13T09:06:40Z

What this PR does:

Add support for SVG color inheritance
Update button styles for disabled state
Improve CsgButton component with disabled prop
Refactor button class handling in RepoClone
Ensure consistent SVG appearance in disabled state

Which issue(s) this PR fixes:

Fixes gitlab 370

Type of changes
Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update

Feature/Issue validation/testing:

Please describe the tests that you ran to verify your changes and relevant result summary.

Test A
Test B
Logs

Special notes for your reviewer:

Checklist:

I have added unit/e2e tests that prove your fix is effective or that this feature works.
I have commented my code, particularly in hard-to-understand areas.
I have made corresponding changes to the documentation.
I have reviewed my own code and ensured that it follows the project's style guidelines.

Release note:

MR Summary:

The summary is added by @codegpt.

The MR enhances SVG handling and the disabled state of buttons in the application. Key updates include:

Added support for SVG color inheritance and ensured consistent SVG appearance in the disabled state.
Updated button styles for the disabled state and improved the CsgButton component with a disabled prop.
Refactored button class handling in RepoClone and added special handling for the disabled state.
Made changes to the RepoCards.vue and RepoClone.vue files to improve UI and UX.
Fixed issue gitlab 370.

- Update gap from 24px to 6 - Change padding-top from 32px to 8 - Modify text color from gray-400 to gray-600 - Set ElInput width to fit instead of fixed

- Add support for SVG color inheritance - Update button styles for disabled state - Improve CsgButton component with disabled prop - Refactor button class handling in RepoClone - Ensure consistent SVG appearance in disabled state

starship-github · 2025-03-13T09:07:19Z

MR Evaluation:

This feature is still under test, evaluation are given by AI and might be inaccurate.

After evaluation, the code changes in the Merge Request get score: 67.

Analysis for the evaluation score:

The code change may not have preprocessed security related exceptions.
The code change may not include corresponding unit tests.
The code change may not include corresponding integration testing.
The code change may not include the corresponding user manual.
The code change may not include the corresponding development documentation.

Tips

CodeReview Commands (invoked as MR or PR comments)

@codegpt /review to trigger an code review.
@codegpt /evaluate to trigger code evaluation process.
@codegpt /describe to regenerate the summary of the MR.
@codegpt /secscan to scan security vulnerabilities for the MR or the Repository.
@codegpt /help to get help.

CodeReview Discussion Chat

There are 2 ways to chat with Starship CodeReview:

Review comments: Directly reply to a review comment made by StarShip.
Example:
- @codegpt How to fix this bug?
Files and specific lines of code (under the "Files changed" tab):
Tag @codegpt in a new review comment at the desired location with your query.
Examples:
- @codegpt generate unit testing code for this code snippet.

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 MR/PR comments.

CodeReview Documentation and Community

Visit our Documentation
for detailed information on how to use Starship CodeReview.

About Us:

Visit the OpenCSG StarShip website for the Dashboard and detailed information on CodeReview, CodeGen, and other StarShip modules.

zhendi

LGTM

- Change input width to 320px - Adjust width for larger screens

- Update customClass prop to class - Map class to className variable - Adjust class checks in computed properties - Simplify button name binding in RepoClone component

hiveer · 2025-03-27T04:03:44Z

frontend/src/assets/stylesheets/element-plus/button.css

  cursor: not-allowed;
  pointer-events: none;
 }

+/* Add SVG color inheritance styles - including deeply nested SVGs */
+.btn svg,
+.btn * svg {


It seems we don't need .btn svg when we have .btn * svg

hiveer · 2025-03-27T04:34:53Z

frontend/src/assets/stylesheets/element-plus/button.css

+.btn svg,
+.btn * svg {
+  fill: currentColor;
+  color: inherit;


Are these two duplicates

hiveer · 2025-03-27T05:48:07Z

frontend/src/assets/stylesheets/element-plus/button.css

-}
-
-
-/* btn-primary */


why remove btn-primary?

hiveer · 2025-03-27T05:50:17Z

frontend/src/components/shared/CsgButton.vue

+  });
+
+  // Map the reserved keyword 'class' to a regular variable 'className'
+  const className = computed(() => props.class);


what if we do not do this conversion? it's also working, right?

hiveer · 2025-03-27T05:55:58Z

frontend/src/components/shared/CsgButton.vue

    :type="btnType"
-    :disabled="loading"
+    :disabled="loading || disabled"


should we add one more check here like:
:disabled="loading || disabled || isDisabledClass"

hiveer · 2025-03-27T06:03:20Z

frontend/src/components/shared/CsgButton.vue

  >
    <i v-if="loading" class="el-icon is-loading"><svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 1024 1024"><path fill="currentColor" d="M512 64a32 32 0 0 1 32 32v192a32 32 0 0 1-64 0V96a32 32 0 0 1 32-32m0 640a32 32 0 0 1 32 32v192a32 32 0 1 1-64 0V736a32 32 0 0 1 32-32m448-192a32 32 0 0 1-32 32H736a32 32 0 1 1 0-64h192a32 32 0 0 1 32 32m-640 0a32 32 0 0 1-32 32H96a32 32 0 0 1 0-64h192a32 32 0 0 1 32 32M195.2 195.2a32 32 0 0 1 45.248 0L376.32 331.008a32 32 0 0 1-45.248 45.248L195.2 240.448a32 32 0 0 1 0-45.248zm452.544 452.544a32 32 0 0 1 45.248 0L828.8 783.552a32 32 0 0 1-45.248 45.248L647.744 692.992a32 32 0 0 1 0-45.248zM828.8 195.264a32 32 0 0 1 0 45.184L692.992 376.32a32 32 0 0 1-45.248-45.248l135.808-135.808a32 32 0 0 1 45.248 0m-452.544 452.48a32 32 0 0 1 0 45.248L240.448 828.8a32 32 0 0 1-45.248-45.248l135.808-135.808a32 32 0 0 1 45.248 0z"></path></svg></i>
    <SvgIcon
      v-if="svgName"
      :name="svgName"
+      :class="['fill-current text-inherit', getIconSizeClass]"
+      :style="{ color: 'currentColor' }"


I think this won't take effect for img element

hiveer · 2025-03-27T06:04:56Z

frontend/src/components/shared/CsgButton.vue

+    color: inherit !important;
+    stroke: currentColor !important;
+  }
+</style>


as we're using embeded way in SvgIcon, like below, the settings for SVG element will not take effect

<img :width="width" :height="height" :src="`/images/${path}/${name}.svg`" onerror="this.style.display='none'" />

youngbeom-shin added 2 commits March 6, 2025 15:49

fix(RepoCards): adjust spacing and styles

f1bec41

- Update gap from 24px to 6 - Change padding-top from 32px to 8 - Modify text color from gray-400 to gray-600 - Set ElInput width to fit instead of fixed

feat(button): enhance SVG handling and disabled state

937def3

- Add support for SVG color inheritance - Update button styles for disabled state - Improve CsgButton component with disabled prop - Refactor button class handling in RepoClone - Ensure consistent SVG appearance in disabled state

youngbeom-shin added the local-verified label Mar 13, 2025

youngbeom-shin self-assigned this Mar 13, 2025

youngbeom-shin requested review from hiveer and zhendi March 13, 2025 09:07

zhendi previously approved these changes Mar 13, 2025

View reviewed changes

fix(RepoCards): update input width styling

03250bf

- Change input width to 320px - Adjust width for larger screens

youngbeom-shin dismissed zhendi’s stale review via 03250bf March 13, 2025 09:22

feat(CsgButton): rename customClass to class

93b016d

- Update customClass prop to class - Map class to className variable - Adjust class checks in computed properties - Simplify button name binding in RepoClone component

youngbeom-shin requested a review from zhendi March 13, 2025 09:38

hiveer added the merge hold label Mar 13, 2025

hiveer requested a review from ymh6315431 March 18, 2025 13:12

hiveer removed the merge hold label Mar 27, 2025

hiveer reviewed Mar 27, 2025

View reviewed changes

frontend/src/assets/stylesheets/element-plus/button.css

}

/* btn-primary */

Copy link

Collaborator

hiveer Mar 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why remove btn-primary?

hiveer reviewed Mar 27, 2025

View reviewed changes

hiveer added the merge hold label Mar 27, 2025

hiveer reviewed Mar 27, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(button): enhance SVG handling and disabled state #1008

feat(button): enhance SVG handling and disabled state #1008

youngbeom-shin commented Mar 13, 2025 •

edited by starship-github bot

Loading

starship-github bot commented Mar 13, 2025

CodeReview Commands (invoked as MR or PR comments)

CodeReview Discussion Chat

CodeReview Documentation and Community

zhendi left a comment

hiveer Mar 27, 2025

hiveer Mar 27, 2025

hiveer Mar 27, 2025

hiveer Mar 27, 2025

hiveer Mar 27, 2025

hiveer Mar 27, 2025

hiveer Mar 27, 2025

feat(button): enhance SVG handling and disabled state #1008

Are you sure you want to change the base?

feat(button): enhance SVG handling and disabled state #1008

Conversation

youngbeom-shin commented Mar 13, 2025 • edited by starship-github bot Loading

MR Summary:

starship-github bot commented Mar 13, 2025

MR Evaluation:

CodeReview Commands (invoked as MR or PR comments)

CodeReview Discussion Chat

CodeReview Documentation and Community

About Us:

zhendi left a comment

Choose a reason for hiding this comment

hiveer Mar 27, 2025

Choose a reason for hiding this comment

hiveer Mar 27, 2025

Choose a reason for hiding this comment

hiveer Mar 27, 2025

Choose a reason for hiding this comment

hiveer Mar 27, 2025

Choose a reason for hiding this comment

hiveer Mar 27, 2025

Choose a reason for hiding this comment

hiveer Mar 27, 2025

Choose a reason for hiding this comment

hiveer Mar 27, 2025

Choose a reason for hiding this comment

youngbeom-shin commented Mar 13, 2025 •

edited by starship-github bot

Loading