Skip to content

Comments

fix: prevented infraMonitoring styles from overriding at other places#7425

Merged
SagarRajput-7 merged 1 commit intomainfrom
SIG-7214
Mar 24, 2025
Merged

fix: prevented infraMonitoring styles from overriding at other places#7425
SagarRajput-7 merged 1 commit intomainfrom
SIG-7214

Conversation

@SagarRajput-7
Copy link
Contributor

@SagarRajput-7 SagarRajput-7 commented Mar 24, 2025

Summary

  • InfraMonitoring Styles had the antd cell override at the global place which effected and messed-up at other places - PlannedDowntime

Related Issues / PR's

Screenshots

Before:

Screen.Recording.2025-03-24.at.8.50.11.PM.mov

After:

Screen.Recording.2025-03-24.at.8.49.13.PM.mov

Affected Areas and Manually Tested Areas


Important

Scopes table cell styles to .infra-monitoring-container in InfraMonitoringK8s.styles.scss to prevent global overrides.

  • Styles:
    • Wraps .ant-table-cell styles within .infra-monitoring-container to prevent global overrides in InfraMonitoringK8s.styles.scss.
    • Moves .ant-table-row-expand-icon-cell styles under .expanded-k8s-list-table within .infra-monitoring-container.
  • Behavior:
    • Ensures table cell styles are scoped to the infra monitoring component, preventing unintended style application elsewhere.

This description was created by Ellipsis for 36aa003. It will automatically update as commits are pushed.

@SagarRajput-7 SagarRajput-7 requested a review from YounixM as a code owner March 24, 2025 15:19
@github-actions github-actions bot added the bug Something isn't working label Mar 24, 2025
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to 36aa003 in 1 minute and 19 seconds

More details
  • Looked at 71 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 7 drafted comments based on config settings.
1. frontend/src/container/InfraMonitoringK8s/InfraMonitoringK8s.styles.scss:711
  • Draft comment:
    Usage of :has() pseudo-class is modern but verify browser support meets target requirements.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the PR author to verify browser support for the :has() pseudo-class, which is not allowed as per the rules. It doesn't provide a specific suggestion or point out a clear issue with the code.
2. frontend/src/container/InfraMonitoringK8s/InfraMonitoringK8s.styles.scss:705
  • Draft comment:
    Good encapsulation: Wrapping the table cell overrides in .infra-monitoring-container limits the scope and prevents leaking styles globally.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
3. frontend/src/container/InfraMonitoringK8s/InfraMonitoringK8s.styles.scss:707
  • Draft comment:
    Consider reducing the use of !important if possible to improve maintainability, unless absolutely necessary to override external library styles.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
4. frontend/src/container/InfraMonitoringK8s/InfraMonitoringK8s.styles.scss:712
  • Draft comment:
    Ensure the browser support for the CSS :has pseudo-class meets our target requirements, as its support can be limited.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to ensure browser support for a CSS feature, which falls under the category of asking the author to double-check or verify something. This violates the rule against asking the author to confirm or ensure things.
5. frontend/src/container/InfraMonitoringK8s/InfraMonitoringK8s.styles.scss:706
  • Draft comment:
    There are multiple separate .ant-table-cell blocks; consider consolidating them to reduce duplication and improve clarity.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
6. frontend/src/container/InfraMonitoringK8s/InfraMonitoringK8s.styles.scss:736
  • Draft comment:
    Double-check that both the nested rule within .expanded-k8s-list-table and the standalone .ant-table-row-expand-icon-cell rule are required to cover all cases without unintended side effects.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
7. frontend/src/container/InfraMonitoringK8s/InfraMonitoringK8s.styles.scss:768
  • Draft comment:
    Typographical error: 'background-color: none !important;' is not a valid CSS value. Consider replacing it with 'background-color: transparent !important;' to ensure correct styling.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_dUqIWsoadOaLMNmA


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@SagarRajput-7 SagarRajput-7 merged commit 9c25a33 into main Mar 24, 2025
14 of 16 checks passed
@SagarRajput-7 SagarRajput-7 deleted the SIG-7214 branch March 24, 2025 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants