WEB-958: Add interaction feedback to account number copy button#3601
Open
RiteshGite wants to merge 1 commit into
Open
WEB-958: Add interaction feedback to account number copy button#3601RiteshGite wants to merge 1 commit into
RiteshGite wants to merge 1 commit into
Conversation
|
Note
|
| Layer / File(s) | Summary |
|---|---|
Semantic button markup and styling src/app/shared/account-number/account-number.component.html, src/app/shared/account-number/account-number.component.scss |
Left and right copy controls replace clickable <span> wrappers with <button mat-icon-button class="copy-btn">, adding translated aria-label and preserving copyValue() with $event.stopPropagation(). .copy-btn SCSS enforces inline-flex centering, removes background/border, sets inherited color with reduced opacity, hides Material ripples, and adds :hover, :active (scale), and :focus-visible (dashed outline) interaction states. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
- IOhacker
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title 'WEB-958: Add interaction feedback to account number copy button' directly and clearly describes the main change: adding visual feedback to the copy button interaction states. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/shared/account-number/account-number.component.html`:
- Around line 11-13: Replace the native <button> copy control with an Angular
Material icon button: change the element that calls copyValue() to use
mat-icon-button and the Material icon component, preserve the
(click)="copyValue(); $event.stopPropagation()", aria-label and translated
title, and ensure the CSS class (e.g., "copy-btn m-l-5") is moved to the
mat-icon-button or adapted to its selector; do the same replacement for the
other copy control referenced at lines 24-26 so both copy controls use
mat-icon-button and mat-icon for proper theming and interaction states.
- Around line 11-13: The aria-label on the copy button is hardcoded English;
replace the literal aria-label value with the translate pipe (use the same
translation key pattern already used for the fa-icon title, e.g. the
'labels.text.Copy Account Number' key) on the button element that calls
copyValue(), and apply the same change to the right-side button (the other copy
button around lines 24-26); after updating both buttons run npm run
translations:extract to update translation files.
In `@src/app/shared/account-number/account-number.component.scss`:
- Around line 9-40: Replace hardcoded color and opacity values in the .copy-btn
selector with the shared SCSS theme variables from src/main.scss and
src/theme/mifosx-theme.scss (e.g., replace opacity: 0.65 and rgb(255 255 255 /
40%) with the corresponding $button-opacity, $muted-opacity or
$focus-outline-color variables used across the app); update
hover/active/focus-visible to use theme-provided interaction values (or remove
them if you switch to a Material button which provides built-in themed states)
so the .copy-btn styling is driven by the central theme variables rather than
literal numbers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 98b4a624-aed6-41f2-b831-045913ba7fe5
📒 Files selected for processing (2)
src/app/shared/account-number/account-number.component.htmlsrc/app/shared/account-number/account-number.component.scss
IOhacker
previously approved these changes
May 25, 2026
RiteshGite
force-pushed
the
WEB-958-account-copy-feedback
branch
from
4aae90a to
da09392
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/app/shared/account-number/account-number.component.html (1)
11-18:⚠️ Potential issue | 🟠 Major | ⚡ Quick winComplete the Material button migration by replacing
<fa-icon>with<mat-icon>.While the template now correctly uses
mat-icon-button, it still renders the FontAwesome icon via<fa-icon>instead of Angular Material's<mat-icon>component. This remains a violation of the guideline requiring Material elements where possible.🎨 Complete the Material icon migration
<button mat-icon-button class="copy-btn m-l-5" (click)="copyValue(); $event.stopPropagation()" [attr.aria-label]="'labels.text.Copy Account Number' | translate" > - <fa-icon icon="copy" size="sm" [title]="'labels.text.Copy Account Number' | translate"></fa-icon> + <mat-icon fontSet="fa" fontIcon="fa-copy"></mat-icon> </button>Apply the same change to the right-side button on lines 29-36.
As per coding guidelines: All visual components must strictly use Angular Material elements (e.g.,
<mat-card>,<mat-select>) instead of native HTML where possible.Also applies to: 29-36
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/shared/account-number/account-number.component.html` around lines 11 - 18, Replace the FontAwesome icon tags with Angular Material icons: in account-number.component.html change the <fa-icon icon="copy" size="sm" [title]="'labels.text.Copy Account Number' | translate"></fa-icon> inside the copy button (the button with class "copy-btn" and (click)="copyValue()") to a Material icon element, e.g. <mat-icon [title]="'labels.text.Copy Account Number' | translate" aria-hidden="false">content_copy</mat-icon>, and apply an appropriate CSS class for sizing if needed; also make the same replacement for the right-side button mentioned in the comment (lines 29-36) so both buttons use <mat-icon> instead of <fa-icon>.
🧹 Nitpick comments (2)
src/app/shared/account-number/account-number.component.scss (2)
41-44: 💤 Low valueAlign focus outline offset with the 8px grid system.
The
outline-offset: 1pxdoes not follow the 8px grid system required by the coding guidelines. Consider using a grid-aligned value or Material's built-in focus indicators.📐 Grid-aligned alternative
&:focus-visible { outline: 1px dashed var(--mdc-outlined-button-outline-color, rgb(255 255 255 / 40%)); - outline-offset: 1px; + outline-offset: 2px; }Note: 2px is closer to grid alignment than 1px, though 8px may be too large for an outline offset in this context.
As per coding guidelines: Stick to the 8px grid system for visual design and spacing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/shared/account-number/account-number.component.scss` around lines 41 - 44, The focus outline offset in account-number.component.scss uses outline-offset: 1px which doesn't align with the project's 8px grid guidelines; update the &:focus-visible rule (in account-number.component.scss) to use a grid-aligned value (e.g., outline-offset: 2px) or replace the custom outline with Material's built-in focus indicator utilities so the focus styling follows the design system.
28-31: ⚡ Quick winReconsider suppressing Material's ripple effect.
Hiding
.mat-mdc-button-persistent-rippleand.mat-rippleremoves a core Material Design interaction affordance. Ripples provide important visual feedback that a button has been activated, especially for users who rely on visual confirmation of their actions.If the ripple truly conflicts with the design intent, consider:
- Using a different Material button variant that better fits the use case
- Customizing the ripple color/opacity rather than removing it entirely
- Documenting why this deviation from Material Design is necessary
Removing standard interaction feedback can negatively impact the user experience and accessibility.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/shared/account-number/account-number.component.scss` around lines 28 - 31, The styles that hide Material ripples (.mat-mdc-button-persistent-ripple and .mat-ripple) remove essential interaction feedback; revert the global display:none and instead scope any visual change to the specific AccountNumber component by removing or narrowing those selectors in account-number.component.scss (delete the display:none rules for .mat-mdc-button-persistent-ripple and .mat-ripple), or replace them with scoped adjustments such as targeting only a specific button class inside the component (e.g., .account-number .my-btn .mat-ripple) and customizing ripple appearance (color/opacity) rather than removing it, or switch to a different Material button variant in the template if the ripple still conflicts; reference the .mat-mdc-button-persistent-ripple and .mat-ripple selectors and the AccountNumber component to locate and apply the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/shared/account-number/account-number.component.scss`:
- Around line 10-13: The .copy-btn rules are using !important to override
Material button defaults (width, height, line-height, padding) which conflicts
with mat-icon-button sizing; remove the !important declarations and either
remove the .copy-btn class entirely to rely on mat-icon-button defaults or keep
.copy-btn but drop !important and, if needed, use a more specific selector
(e.g., targetting mat-icon-button.copy-btn) to apply only the minimal custom
styles you actually require (opacity, color, etc.) so Material's built-in sizing
remains intact.
---
Duplicate comments:
In `@src/app/shared/account-number/account-number.component.html`:
- Around line 11-18: Replace the FontAwesome icon tags with Angular Material
icons: in account-number.component.html change the <fa-icon icon="copy"
size="sm" [title]="'labels.text.Copy Account Number' | translate"></fa-icon>
inside the copy button (the button with class "copy-btn" and
(click)="copyValue()") to a Material icon element, e.g. <mat-icon
[title]="'labels.text.Copy Account Number' | translate"
aria-hidden="false">content_copy</mat-icon>, and apply an appropriate CSS class
for sizing if needed; also make the same replacement for the right-side button
mentioned in the comment (lines 29-36) so both buttons use <mat-icon> instead of
<fa-icon>.
---
Nitpick comments:
In `@src/app/shared/account-number/account-number.component.scss`:
- Around line 41-44: The focus outline offset in account-number.component.scss
uses outline-offset: 1px which doesn't align with the project's 8px grid
guidelines; update the &:focus-visible rule (in account-number.component.scss)
to use a grid-aligned value (e.g., outline-offset: 2px) or replace the custom
outline with Material's built-in focus indicator utilities so the focus styling
follows the design system.
- Around line 28-31: The styles that hide Material ripples
(.mat-mdc-button-persistent-ripple and .mat-ripple) remove essential interaction
feedback; revert the global display:none and instead scope any visual change to
the specific AccountNumber component by removing or narrowing those selectors in
account-number.component.scss (delete the display:none rules for
.mat-mdc-button-persistent-ripple and .mat-ripple), or replace them with scoped
adjustments such as targeting only a specific button class inside the component
(e.g., .account-number .my-btn .mat-ripple) and customizing ripple appearance
(color/opacity) rather than removing it, or switch to a different Material
button variant in the template if the ripple still conflicts; reference the
.mat-mdc-button-persistent-ripple and .mat-ripple selectors and the
AccountNumber component to locate and apply the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 414df4c2-ed3b-4736-9a00-eeade0650547
📒 Files selected for processing (2)
src/app/shared/account-number/account-number.component.htmlsrc/app/shared/account-number/account-number.component.scss
RiteshGite
force-pushed
the
WEB-958-account-copy-feedback
branch
from
da09392 to
d8be7e4
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/app/shared/account-number/account-number.component.scss (2)
31-31: ⚡ Quick winAlign outline-offset to the 8px grid system.
The
outline-offset: 1pxuses an explicit pixel value that doesn't align with the 8px grid system required by the coding guidelines.📐 Proposed fix
- outline-offset: 1px; + outline-offset: 0;As per coding guidelines: Stick to the 8px grid system for visual design and spacing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/shared/account-number/account-number.component.scss` at line 31, The outline-offset: 1px in account-number.component.scss doesn't follow the 8px grid; change the value to an 8px-grid-aligned spacing (e.g. 8px or the design token used in the project like $spacing-8 or var(--spacing-8)) for the selector that sets outline-offset so the component aligns with the 8px spacing system.
16-19: ⚖️ Poor tradeoffReconsider hiding Material's ripple effect.
Hiding
.mat-mdc-button-persistent-rippleand.mat-rippleremoves Angular Material's built-in interaction feedback, which is a core accessibility and UX feature. While custom hover/active states are added, Material's ripple provides standardized, tested interaction feedback that enhances accessibility.If the ripple visual conflicts with the design intent, consider adjusting ripple color/opacity rather than removing it entirely.
💡 Alternative approach
- .mat-mdc-button-persistent-ripple, - .mat-ripple { - display: none; - }Remove these lines to preserve Material's ripple, or if color adjustment is needed:
.mat-mdc-button-persistent-ripple, .mat-ripple { opacity: 0.3; // Adjust ripple visibility instead of hiding }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/shared/account-number/account-number.component.scss` around lines 16 - 19, The stylesheet currently disables Angular Material's interaction feedback by setting .mat-mdc-button-persistent-ripple and .mat-ripple to display: none; restore accessible ripple feedback by removing or reverting that rule and, if visual conflict exists, modify the ripple appearance instead (e.g., adjust opacity/color) rather than hiding it entirely; update the rule that targets .mat-mdc-button-persistent-ripple and .mat-ripple in account-number.component.scss to either delete those selectors or change their properties to tweak visibility (opacity/color) so Material's ripple remains available for UX/accessibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/app/shared/account-number/account-number.component.scss`:
- Line 31: The outline-offset: 1px in account-number.component.scss doesn't
follow the 8px grid; change the value to an 8px-grid-aligned spacing (e.g. 8px
or the design token used in the project like $spacing-8 or var(--spacing-8)) for
the selector that sets outline-offset so the component aligns with the 8px
spacing system.
- Around line 16-19: The stylesheet currently disables Angular Material's
interaction feedback by setting .mat-mdc-button-persistent-ripple and
.mat-ripple to display: none; restore accessible ripple feedback by removing or
reverting that rule and, if visual conflict exists, modify the ripple appearance
instead (e.g., adjust opacity/color) rather than hiding it entirely; update the
rule that targets .mat-mdc-button-persistent-ripple and .mat-ripple in
account-number.component.scss to either delete those selectors or change their
properties to tweak visibility (opacity/color) so Material's ripple remains
available for UX/accessibility.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f1a4be3a-7f21-4d34-a05f-c08a80848a44
📒 Files selected for processing (2)
src/app/shared/account-number/account-number.component.htmlsrc/app/shared/account-number/account-number.component.scss
RiteshGite
force-pushed
the
WEB-958-account-copy-feedback
branch
from
d8be7e4 to
1060722
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds visible interaction feedback to the account number copy button.
Previously, when users clicked the copy button, there was no clear hover or click feedback, so it was difficult to understand whether the account number had actually been copied or not.
This update improves the user interaction experience by adding visible interaction states to the copy control.
Related issues and discussion
#WEB-958
Screenshots, if any
Before
Before.copied.feedback.not.mp4
After
After.copied.feedback.mp4
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
If you have multiple commits please combine them into one commit by squashing them.
Read and understood the contribution guidelines at
web-app/.github/CONTRIBUTING.md.Summary by CodeRabbit
Accessibility
User Interface