-
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
fix scss linting warnings #1494
Conversation
WalkthroughThe pull request introduces significant updates to the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (6)app/styles/_component-variables.scss (6)
The reorganization of ak-appbar variables into theme-specific blocks improves maintainability and follows a consistent naming pattern.
The new variables follow consistent naming conventions and use appropriate base variables for colors, borders, and other properties.
The variables are well-organized, follow consistent naming patterns, and properly reference base theme variables.
The variables follow the established naming pattern and properly reference base theme variables.
The variables maintain consistency with the codebase's naming conventions and properly reference base theme variables.
The variables are properly organized within theme selectors and maintain consistency in naming and value references. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
app/styles/_component-variables.scss (4)
330-481
: Consolidate Multiplebody
Selectors for MaintainabilityMultiple
body
selectors are defined (lines 1~, 331~, and 485~). Consider consolidating these variable declarations within a singlebody
selector or organizing them under appropriate theme or component classes to improve code maintainability and readability.
1808-1814
: Maintain Consistency Between Static and Dynamic Scan VariablesVariables for
file-details/static-scan
(lines 1809~ to 1814~) are defined separately from those fordynamic-scan
. Ensure that the naming conventions and structure for static scan variables are consistent with those used for dynamic scans to promote clarity and ease of maintenance.
1840-1841
: Confirm Notification Bell Icon Colors in Light ThemeIn the light theme (lines 1840~ to 1841~), the variables for the notification bell icon are:
--notification-bell-icon-dot-border-color: var(--background-main); --notification-bell-icon-color: var(--text-primary);Ensure that these colors provide adequate visibility against the background and that the notification dot is clearly distinguishable.
1846-1847
: Review Notification Bell Icon Colors in Dark ThemeSimilarly, in the dark theme (lines 1846~ to 1847~), verify that the notification bell icon colors are appropriate:
--organization-dashboard-header-background-color: var(--secondary-main); --organization-dashboard-header-text-color: var(--secondary-contrast-text);Ensure the bell icon and text are visible against the secondary main background color.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
app/styles/_component-variables.scss
(7 hunks)
🔇 Additional comments (4)
app/styles/_component-variables.scss (4)
312-326
: Confirm Theme-specific Variable Usage
The variables for --ak-appbar-inherit-*
and --ak-appbar-box-shadow
are defined differently within .theme-light
and .theme-dark
classes. Ensure that components correctly apply these theme-specific variables to render the appropriate styles in light and dark modes.
If necessary, review the components that use these variables to confirm they are accessing the theme-scoped variables.
768-770
: Ensure Correct Theme Colors for Vulnerabilities List
In the variables for sbom/component-details/vulnerabilities
(lines 768~ to 770~), verify that the colors assigned to --sbom-component-details-vulnerabilities-list-header-border-color-dark
and --sbom-component-details-vulnerabilities-list-border-color-light
correctly correspond to the dark and light themes, respectively.
Confirm that these variables are used appropriately in both themes and adjust if necessary.
Line range hint 1460-1464
: Correct Background Color Variable for Recent Issues
At line 1460~, the variable --organization-analytics-recent-issues-background-color
is set to var(--neutral-grey-200)
. Verify if this is the intended background color, as it might be too light for certain themes, potentially affecting readability.
Ensure that the background color provides sufficient contrast with text and is consistent with the overall theme.
1-14
:
Verify the Removal of Old App Bar Variables Across the Codebase
The variables --ak-appbar-default-*
, --ak-appbar-light-*
, and --ak-appbar-dark-*
have been removed and replaced with --ak-appbar-inherit-*
. Please ensure that all references to the old variables have been updated to use the new variables to prevent any styling issues.
Run the following script to identify any remaining usages of the removed variables:
927717d
to
8b24b83
Compare
Deploying irenestaging with Cloudflare Pages
|
Quality Gate passedIssues Measures |
Irene Run #529
Run Properties:
|
Project |
Irene
|
Branch Review |
PD-1584-fix-scss-linting-warnings
|
Run status |
Failed #529
|
Run duration | 05m 03s |
Commit |
f78716d78d ℹ️: Merge 927717dbd004d90f4e1ddc8af77687af827af19b into 1a8d4e75c39289011914d4425709...
|
Committer | Avi Shah |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
3
|
Flaky |
0
|
Pending |
0
|
Skipped |
4
|
Passing |
25
|
View all changes introduced in this branch ↗︎ |
Tests for review
dynamic-scan.spec.ts • 2 failed tests
Test | Artifacts | |
---|---|---|
Dynamic Scan > it tests dynamic scan for an apk file: 58062 |
Test Replay
Screenshots
|
|
Dynamic Scan > it tests dynamic scan for an ipa file: 58061 |
Test Replay
Screenshots
|
service-account.spec.ts • 1 failed test
Test | Artifacts | |
---|---|---|
Service Account > should create service account with expiry & delete it |
Test Replay
Screenshots
|
No description provided.