-
Notifications
You must be signed in to change notification settings - Fork 584
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: UI Issue in Option Translations Modal #8878
Conversation
WalkthroughThe pull request updates the product option group translation component by restructuring the HTML and overhauling the corresponding SCSS. In the HTML file, input fields are now wrapped in a container that includes a new language label, while the SCSS file has been refactored to replace outdated classes with a new structure featuring a main container and nested styles for card elements and input fields. No changes were made to the underlying functionality or public entities. Changes
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
🪧 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: 0
🧹 Nitpick comments (3)
packages/ui-core/shared/src/lib/product/product-mutation/product-option-group-translation/product-option-group-translation.component.html (2)
23-32
: Enhanced Group Translation Input Container StructureThe new
<div class="input-container">
along with the nested<span class="language-label">
and<input>
provides a clear, semantic grouping for active group translation fields. This layout improves visual hierarchy and aligns well with the updated UI strategy.Note: Consider generating unique IDs for the
<input>
element (currently statically set asnameInput
) if multiple instances can render on the page, to avoid accessibility issues or conflicts in the DOM.
48-57
: Enhanced Option Translation Input Container StructureSimilar to the group section, wrapping the active option translation input within an
<div class="input-container">
with an accompanying<span class="language-label">
ensures a consistent and improved UI layout. The conditional rendering still preserves the intended behavior based onisOptionActive(option, ln.value)
.Note: As above, verify that each input’s
id
remains unique if multiple inputs are rendered simultaneously.packages/ui-core/shared/src/lib/product/product-mutation/product-option-group-translation/product-option-group-translation.component.scss (1)
131-155
: New Input Container StylingThe styling for the
.input-container
delivers a robust flex layout for aligning the language label and input field, along with clear borders and background settings for the label. This addition supports the structural HTML changes well.Note: Consider if the use of
!important
on properties for the innerinput
(lines 151-153) is necessary. If possible, revising the CSS specificity to avoid!important
might lead to better maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/ui-core/shared/src/lib/product/product-mutation/product-option-group-translation/product-option-group-translation.component.html
(2 hunks)packages/ui-core/shared/src/lib/product/product-mutation/product-option-group-translation/product-option-group-translation.component.scss
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build
- GitHub Check: test
🔇 Additional comments (1)
packages/ui-core/shared/src/lib/product/product-mutation/product-option-group-translation/product-option-group-translation.component.scss (1)
3-30
: Updated Main Container StylingThe newly introduced
.main
class effectively applies a modern look to the component with a subtle box-shadow, rounded borders, and controlled overflow. Nested styles for the card header (including the cancel button and title) are organized clearly.Note: Double-check that these styles do not unintentionally override or conflict with global card styles elsewhere in the application.
View your CI Pipeline Execution ↗ for commit 5227b39.
☁️ Nx Cloud last updated this comment at |
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.
PR Summary
This PR enhances the UI of the Option Translations Modal by improving input field organization and visual hierarchy through consistent styling and layout patterns.
- Added
.input-container
wrapper with language labels for better visual grouping in/packages/ui-core/shared/src/lib/product/product-mutation/product-option-group-translation/product-option-group-translation.component.html
- Implemented modern card-based layout with proper shadows and borders in
/packages/ui-core/shared/src/lib/product/product-mutation/product-option-group-translation/product-option-group-translation.component.scss
- Added responsive design considerations for mobile views with
@media
queries - Improved hover effects and interactive states for better user feedback
💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!
2 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile
...ct-mutation/product-option-group-translation/product-option-group-translation.component.html
Outdated
Show resolved
Hide resolved
...ct-mutation/product-option-group-translation/product-option-group-translation.component.html
Outdated
Show resolved
Hide resolved
...ct-mutation/product-option-group-translation/product-option-group-translation.component.scss
Outdated
Show resolved
Hide resolved
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: 0
🧹 Nitpick comments (2)
packages/ui-core/shared/src/lib/product/product-mutation/product-option-group-translation/product-option-group-translation.component.scss (2)
56-79
: Refine Option Line ElementThe
.option-line
styling is clean with proper hover effects and spacing. However, note that the inner.language-span
is still defined here even though the updated UI uses a new.language-label
for language display. Please verify if this legacy class is still required. If it’s no longer in use, consider removing or renaming it to maintain consistency across the component.- .language-span { - flex-shrink: 0; - font-weight: 600; - color: #495057; - margin-right: 0.5rem; - padding-right: 0.5rem; - border-right: 1px solid #e9ecef; - display: inline-block; - }
156-160
: Consider Future-Proofing ::ng-deep UsageThe
:host ::ng-deep
rule effectively overrides thenb-input
padding, which is necessary for current styling needs. Note, however, that the::ng-deep
combinator is deprecated and may be removed in future Angular versions. It might be worthwhile to explore alternative encapsulation strategies to future-proof the styling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/ui-core/shared/src/lib/product/product-mutation/product-option-group-translation/product-option-group-translation.component.html
(2 hunks)packages/ui-core/shared/src/lib/product/product-mutation/product-option-group-translation/product-option-group-translation.component.scss
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ui-core/shared/src/lib/product/product-mutation/product-option-group-translation/product-option-group-translation.component.html
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build
- GitHub Check: test
🔇 Additional comments (7)
packages/ui-core/shared/src/lib/product/product-mutation/product-option-group-translation/product-option-group-translation.component.scss (7)
3-7
: .main Container Styling Strengthens the Visual HierarchyThe new
.main
container applies a consistent box-shadow, border-radius, and overflow setting, which enhances the dialog’s visual containment and overall polish.
8-29
: Well-Structured Header SectionThe
nb-card-header
block now clearly defines spacing and positioning. The use of relative positioning in the header paired with the absolutely positioned.cancel
button is appropriate, ensuring the cancel icon displays as intended.
31-55
: Responsive Card Body LayoutThe updated
.option-row
and the nested.col-md-3
rules effectively introduce a flexible, responsive layout. The use of negative margins and media queries ensures proper alignment across devices.
82-91
: Input Field Styling Looks ConsistentThe input fields within the card body now have clear styling with full-width and defined padding. The special rule for
[nbInput]
ensures that inputs flagged by this attribute receive adjusted padding.
94-112
: Creative Use of Pseudo-Elements for Form ControlsThe use of a
::before
pseudo-element with[formControlName]
to simulate a label is a smart approach to maintain a clean UI. Just ensure that the attribute case (formcontrolname
vs.formControlName
) remains consistent with Angular conventions for better maintainability.
115-128
: Solid Footer LayoutThe
nb-card-footer
block applies padding and a border-top effectively, and the spacing applied to buttons (especially using:first-child
) is balanced and clear.
130-154
: Robust Input Container ImplementationThe
.input-container
with its flex layout, border, and rounded corners provides a cohesive structure for the input elements. The new.language-label
class is well-defined, ensuring that language identifiers are clearly presented above or beside the inputs.
* fix: UI Issue in Option Translations Modal (#8878) * fix: UI Issue in Option Translations Modal * feedback integration * [Fix] Issues with Accounting Tab (Invoice, Estimate, and Receipt Templates) (#8880) * fix(accounting-template): resolve duplicate templates and incorrect language filtering * Remove console log * feedback integration * Remove duplicated comment * [Fix] Inventory Filters (#8881) * fix:Inventory Filters * fix: remove filter by name * feedback Integration * [Enhancement] Introduce SELECT_EMPLOYEE Permission & Allow Employee to Assign Managers/Members in Project Creation (#8836) * feat: allows employees to select others * chore: add translations for SELECT_EMPLOYEE permission * Fix: retrieved employees * Integration of feedback * Fix: remove CHANGE_SELECTED_EMPLOYEE permission * Fix: Ensure both CHANGE_SELECTED_EMPLOYEE and SELECT_EMPLOYEE permissions are applied --------- Co-authored-by: mbabhaziSamuel <[email protected]> --------- Co-authored-by: Samuel Mbabhazi <[email protected]> Co-authored-by: samuel mbabhazi <[email protected]>
PR
Please note: we will close your PR without comment if you do not check the boxes above and provide ALL requested information.
Summary by CodeRabbit
New Features
Style