-
Notifications
You must be signed in to change notification settings - Fork 46
Ccsd 595 #3007
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
base: develop
Are you sure you want to change the base?
Ccsd 595 #3007
Changes from all commits
48af629
b446e6a
a74e9cd
d895ae3
b36a24c
1b2202c
1ea9458
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -550,6 +550,11 @@ digit-card-text.center { | |||||||||||||||||
text-align: center; | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
.employee-app-wrapper .sandbox-module { | ||||||||||||||||||
min-height: calc(100vh - 4em); | ||||||||||||||||||
margin: -70px; | ||||||||||||||||||
} | ||||||||||||||||||
Comment on lines
+553
to
+556
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Optional: consider a token/variable for header height instead of hardcoded 4em If a design token or CSS variable exists for topbar height, prefer it to keep layouts consistent across themes. Example: min-height: calc(100vh - var(--digit-topbar-height, 4em)); 🤖 Prompt for AI Agents
🛠️ Refactor suggestion Selector will never match with current DOM; also avoid sweeping negative margins You add both classes to the same element in index.js, but the selector here uses a descendant combinator. Fix the selector to target the same element and constrain the negative margin to the top only. Apply this diff: -.employee-app-wrapper .sandbox-module {
- min-height: calc(100vh - 4em);
- margin: -70px;
-}
+.employee-app-wrapper.sandbox-module {
+ min-height: calc(100vh - 4em);
+ margin-top: -70px;
+} Note: This preserves the intended vertical offset while preventing unintended horizontal overflow. The earlier learning about intentional layout shifts in sandbox styling is respected; this change only scopes the effect correctly. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||
|
||||||||||||||||||
.pgr-citizen-wrapper { | ||||||||||||||||||
.applications-list-container { | ||||||||||||||||||
.card { | ||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -133,7 +133,7 @@ const EmployeeApp = ({ | |||||||||||||||||||||||
/>} | ||||||||||||||||||||||||
<div className={!noTopBar ? `${(isSuperUserWithMultipleRootTenant) ? "" : "main"} ${DSO ? "m-auto" : ""} digit-home-main` : ""}> | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
<div className={!noTopBar ? `${(isSuperUserWithMultipleRootTenant && hideClass) ? "" : "employee-app-wrapper"} digit-home-app-wrapper` : ""}> | ||||||||||||||||||||||||
<div className={!noTopBar ? `${(isSuperUserWithMultipleRootTenant && hideClass) ? "" : "employee-app-wrapper sandbox-module"} digit-home-app-wrapper` : ""}> | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Optional: simplify conditional class composition Consider using a helper like Example with import clsx from "clsx";
// ...
<div
className={
!noTopBar
? clsx(
"digit-home-app-wrapper",
"employee-app-wrapper",
{ "sandbox-module": !(isSuperUserWithMultipleRootTenant && hideClass) }
)
: ""
}
> 🤖 Prompt for AI Agents
💡 Verification agent 🧩 Analysis chainClasses added on the same element; ensure CSS selector matches and keep base wrapper class when hiding sandbox modifier You’re applying Apply this diff to keep the base class present: - <div className={!noTopBar ? `${(isSuperUserWithMultipleRootTenant && hideClass) ? "" : "employee-app-wrapper sandbox-module"} digit-home-app-wrapper` : ""}>
+ <div className={!noTopBar ? `${(isSuperUserWithMultipleRootTenant && hideClass) ? "employee-app-wrapper" : "employee-app-wrapper sandbox-module"} digit-home-app-wrapper` : ""}> Action items to verify:
🏁 Script executed: #!/bin/bash
# Display the relevant JSX snippet from the employee page index.js
sed -n '130,150p' micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/index.js || true
# Search SCSS/CSS for definitions of sandbox-module and employee-app-wrapper
rg -n "\.sandbox-module" -g '*.{scss,css}'
rg -n "\.employee-app-wrapper" -g '*.{scss,css}' Length of output: 1829 Ensure base wrapper class is always present and update SCSS selector The JSX currently drops • File: • SCSS: • JSX diff: - <div className={!noTopBar ? `${(isSuperUserWithMultipleRootTenant && hideClass) ? "" : "employee-app-wrapper sandbox-module"} digit-home-app-wrapper` : ""}>
+ <div className={!noTopBar ? `${(isSuperUserWithMultipleRootTenant && hideClass) ? "employee-app-wrapper" : "employee-app-wrapper sandbox-module"} digit-home-app-wrapper` : ""}> 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||
{/* <div className="employee-app-wrapper digit-home-app-wrapper"> */} | ||||||||||||||||||||||||
<ErrorBoundary initData={initData}> | ||||||||||||||||||||||||
<AppModules | ||||||||||||||||||||||||
|
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.
🧹 Nitpick (assertive)
Fix hyphen/spacing and clarify entry
There’s a missing space after the bullet and double spacing in the sentence. Suggested clearer phrasing:
📝 Committable suggestion
🧰 Tools
🪛 LanguageTool
[grammar] ~3-~3: Use hyphens correctly
Context: ...# Changelog ## [1.8.39] - [18-Aug-2025] -Employee app wrapper classname css added...
(QB_NEW_EN_OTHER_ERROR_IDS_29)
[grammar] ~4-~4: Use correct spacing
Context: ...mployee app wrapper classname css added margin fixed for sandbox ## [1.8.38] - ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~4-~4: There might be a mistake here.
Context: ...name css added margin fixed for sandbox ## [1.8.38] - [14-Aug-2025] -Removed the lo...
(QB_NEW_EN_OTHER)
🤖 Prompt for AI Agents