One button system: adopt the four roles and gate them in CI - #1687
Merged
Conversation
Converts 16 dialog buttons to the four roles the frontend defines — .btn-primary, .btn-cancel, .btn-delete, .btn-quiet. Every id, binding, label and button order preserved; only the Material directive and its inert `color` input are removed. Role follows the label and intent, not the old colour. Also adds the gate to both workflows, which is the part that keeps this fixed. The checker lives in the frontend repo, but a frontend CI run cannot see these templates — src/app/plugins/ is gitignored there — so without this step nothing checks time-planning-pn's buttons and they drift straight back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015sXLtgzZU8QL9m84GqMkoJ
There was a problem hiding this comment.
Pull request overview
Updates the Time Planning plugin’s dialog templates to use the standardized “four button roles” CSS classes and adds a CI gate (via the frontend repo’s checker) to prevent button-style drift in plugin templates.
Changes:
- Replaced Angular Material button directives (
mat-button/mat-raised-button+color) with role classes (btn-primary,btn-cancel,btn-delete) in multiplemat-dialog-actionstemplates. - Added a “Button conventions” CI step to both PR and master workflows to run the shared checker against this plugin’s templates.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| eform-client/src/app/plugins/modules/time-planning-pn/modules/working-hours/components/working-hours-actions/working-hours-comment-office-all-update-modal/working-hours-comment-office-all-update-modal.component.html | Converts dialog action buttons to role classes. |
| eform-client/src/app/plugins/modules/time-planning-pn/modules/registration-devices/components/registration-devices-actions/registration-devices-otp-code/registration-devices-otp-code.component.html | Converts dialog action buttons to role classes. |
| eform-client/src/app/plugins/modules/time-planning-pn/modules/flexes/components/time-flexes-actions/comment-office-all-update-modal/time-flexes-comment-office-all-update-modal.component.html | Converts dialog action buttons to role classes. |
| eform-client/src/app/plugins/modules/time-planning-pn/modules/break-policies/components/break-policy-rule-dialog/break-policy-rule-dialog.component.html | Converts dialog action buttons to role classes. |
| eform-client/src/app/plugins/modules/time-planning-pn/modules/break-policies/components/break-policies-delete-modal/break-policies-delete-modal.component.html | Converts delete/cancel buttons to role classes. |
| eform-client/src/app/plugins/modules/time-planning-pn/modules/break-policies/components/break-policies-create-modal/break-policies-create-modal.component.html | Converts create/cancel buttons to role classes. |
| eform-client/src/app/plugins/modules/time-planning-pn/modules/absence-requests/components/absence-requests-actions/absence-requests-reject-modal/absence-requests-reject-modal.component.html | Converts reject/cancel buttons to role classes. |
| eform-client/src/app/plugins/modules/time-planning-pn/modules/absence-requests/components/absence-requests-actions/absence-requests-approve-modal/absence-requests-approve-modal.component.html | Converts approve/cancel buttons to role classes. |
| .github/workflows/dotnet-core-pr.yml | Adds button convention checker step in PR workflow. |
| .github/workflows/dotnet-core-master.yml | Adds button convention checker step in master workflow. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
23
to
+31
| - name: Copy dependencies | ||
| run: | | ||
| cp -av eform-angular-timeplanning-plugin/eform-client/src/app/plugins/modules/time-planning-pn eform-angular-frontend/eform-client/src/app/plugins/modules/time-planning-pn | ||
| cd eform-angular-frontend/eform-client && ../../eform-angular-timeplanning-plugin/testinginstallpn.sh | ||
| - name: Button conventions | ||
| # The checker lives in the frontend repo (checked out above); these | ||
| # templates are ours. A frontend CI run cannot see them because | ||
| # src/app/plugins/ is gitignored there, so this plugin has to run it. | ||
| run: cd eform-angular-frontend/eform-client && node scripts/check-button-conventions.js src/app/plugins/modules/time-planning-pn |
Comment on lines
27
to
+35
| - name: Copy dependencies | ||
| run: | | ||
| cp -av eform-angular-timeplanning-plugin/eform-client/src/app/plugins/modules/time-planning-pn eform-angular-frontend/eform-client/src/app/plugins/modules/time-planning-pn | ||
| cd eform-angular-frontend/eform-client && ../../eform-angular-timeplanning-plugin/testinginstallpn.sh | ||
| - name: Button conventions | ||
| # The checker lives in the frontend repo (checked out above); these | ||
| # templates are ours. A frontend CI run cannot see them because | ||
| # src/app/plugins/ is gitignored there, so this plugin has to run it. | ||
| run: cd eform-angular-frontend/eform-client && node scripts/check-button-conventions.js src/app/plugins/modules/time-planning-pn |
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
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.
Plugin half of the button consolidation. Depends on microting/eform-angular-frontend#8016, already merged to
stable, which defines the roles and ships the checker.Audit: Button Drift
What changes
16 dialog buttons move to the four roles:
.btn-primary(filled),.btn-cancel(outlined),.btn-delete(destructive),.btn-quiet(text). Everyid, binding, tooltip, label and button order is preserved — only the Material directive and itscolorinput go, andcolorwas inert on a plain button anyway.Role follows the label and intent, not the old colour. Several confirms carried
color="warn"without being destructive and are.btn-primary.Why the CI step matters more than the conversion
The checker lives in the frontend repo, but a frontend CI run cannot see these templates —
src/app/plugins/is gitignored there, so its checkout contains only the core app. The frontend PR fixed 55 violations in core and was structurally incapable of seeing the 91 in the plugins.So each plugin repo has to run the checker over its own module, or nothing checks its buttons and they drift straight back. That step is added to both workflows here.
What it enforces
Inside a
mat-dialog-actionsrow, every button must carry one of the four roles; no template may reference a class no stylesheet defines. Material's button directives are rejected in both spellings — the theme override targeted.mat-mdc-text-button, which Angular Material 20 never emits, somat-buttonrendered as a pill in the wrong colour and always had.🤖 Generated with Claude Code
https://claude.ai/code/session_015sXLtgzZU8QL9m84GqMkoJ