NIFI-15951 modularize customized angular material themed components#11262
NIFI-15951 modularize customized angular material themed components#11262scottyaslan wants to merge 1 commit into
Conversation
|
reviewing... |
| } | ||
| } | ||
|
|
||
| .darkMode { |
There was a problem hiding this comment.
The .darkMode { ... } block on lines 69–115 is byte-for-byte identical to the :root { ... } block on lines 21–67 — every value uses --mat-sys-* or --nf-* custom properties, all of which are already rebound under .darkMode at the material.scss level. Duplicating the rules under a .darkMode selector emits the same CSS twice and doesn't change any computed values.
This appears to be the most visible artifact of porting the openflow-ui runtime's per-partial pattern — runtime needs separate .darkMode blocks because Balto tokens sometimes need different values per mode at the consumer level, but NiFi's mode-aware values are reassigned at the variable level inside material.scss, so any rule that uses one of those custom properties is automatically mode-aware.
The same redundancy exists in:
_drag-and-drop.scss(lines 84–106)_option.scss(lines 28–32)_paginator.scss(lines 27–31)_progress-bar.scss(lines 47–71)_searchable-overlay.scss(lines 93–107)_skeleton.scss(lines 27–31)_tree.scss(lines 28–32)_typography.scss(lines 134–140)
Suggest dropping the .darkMode { ... } block from each of these partials. Files where the dark-mode block is genuinely needed (different raw palette indices per mode) and should stay: _button.scss, _checkbox.scss, _progress-spinner.scss, _snackbar.scss.
| } | ||
| } | ||
|
|
||
| .darkMode { |
There was a problem hiding this comment.
Light/dark asymmetry: the :root block sets display: inline !important; (line 23) but the .darkMode block omits it — so a mat-tree rendered with a .darkMode ancestor loses the display: inline override.
Either restore symmetry, or (preferred, see the broader .darkMode redundancy comment on _status-variant.scss) drop the .darkMode { ... } block entirely since both :root rules use no color tokens that vary by mode.
| @mixin generate-material-theme() { | ||
| :root { | ||
| mat-datepicker-toggle .mat-mdc-icon-button.mat-mdc-button-base.mdc-icon-button { | ||
| padding: 11px; |
There was a problem hiding this comment.
The original _app.scss had an inline comment explaining this magic number: padding: 11px; //center the mat-icon-button inside the date picker form field. That comment was dropped during the move — please restore it so 11px doesn't read as an unexplained magic number.
|
|
||
| @mixin generate-material-theme() { | ||
| :root { | ||
| .mat-mdc-option[disabled] { |
There was a problem hiding this comment.
General observation about the .mat-mdc-option narrowing called out in the PR description.
The PR description lists narrowing .mat-mdc-option's general sizing/padding (min-height, padding, font-weight, .fa icon sizing) from global scope to .searchable-overlay as an Intentional Scope Change, on the basis that the previously-global rules were "tightly coupled to the searchable-overlay pattern." That assumption appears to be incorrect empirically — testing the resulting build, the two <mat-select> dropdowns on the Scheduling tab of the Processor Configuration dialog now render with visibly different option padding/sizing than before. They aren't searchable selects and don't carry the .searchable-overlay class wrapper, so they no longer pick up the min-height: 32px; padding: 8px 12px; ... rules that were globally applied before.
Almost certainly there are other surfaces in the same situation — any non-searchable <mat-select> / <mat-autocomplete> panel in the controller-services view, parameter-contexts dialog, processor properties drawer, etc. The same concern probably applies to the .mat-mdc-form-field-icon-prefix .fa and outlined-form-field padding that were narrowed for the same reason; an audit of <mat-select> usages outside .searchable-overlay is worth doing before merge.
Two ways to fix without giving up the modularization win:
- Re-add the general
.mat-mdc-option { min-height: 32px; padding: 8px 12px; font-weight: 400; ... }rules to this partial's:rootblock (alongside the existing[disabled]rule), and let.searchable-overlayfurther narrow as needed — most of its current values are the same anyway, so the override would mostly just be reinstating the global baseline. - Keep
_searchable-overlay.scssas the only owner of those rules and explicitly walk the app to apply the.searchable-overlayclass wrapper to every panel that should pick them up — likely a much larger and riskier change than (1).
(1) seems lower-risk for a refactor PR and matches the spirit of "behavior-preserving modularization." Recommend doing it before merge so we don't ship a visual regression.
Before PR:
After:
Summary
NIFI-15951
Summary
Decomposes the monolithic
material.scssand_app.scsstheme files into 22 per-component SCSSpartials under
themes/components/. Each partial exposes agenerate-material-theme()mixin thatis called from the bottom of
material.scss.Intentional Scope Changes
The following styles were intentionally narrowed or removed as part of this refactoring:
mat.select-overrides()(trigger text size, line-height, tracking): Removed from globalscope. These overrides were tightly coupled to the searchable-overlay pattern and are no longer
applied globally to all
mat-selectinstances..mat-mdc-select-panel(margin-top, border, border-radius): Removed from global scope.Select panel border styling is now scoped to
.searchable-overlayin_searchable-overlay.scss..mat-mdc-optiongeneral styling (min-height, padding, font-weight,.faicon sizing):Narrowed to
.searchable-overlayscope. The typography-related option styles (font-size,letter-spacing, line-height) remain global within
.mat-typography.text-basein_typography.scss..mat-mdc-form-field-icon-prefix .faand form-field padding (.mdc-text-field--outlined,.mat-mdc-form-field-has-icon-prefix): Narrowed from global scope to.searchable-overlayandsearchable-select.component.scssrespectively, as these paddings were only relevant tosearchable components.
Bug Fix
--themed-reusable-text-primary: The original_app.scssreferenced this CSS custom propertyfor
.mat-mdc-form-field-icon-prefix .facolor, but this is a Balto/openflow-ui token that doesnot exist in NiFi. Corrected to
var(--mat-app-text-color)in_searchable-overlay.scss.Purple Theme Simplification
The purple theme previously duplicated component overrides (snackbar, checkbox, spinner, icon-button,
error-button) that were identical to the default theme. These have been removed. The purple theme now
only imports
dialogas a demonstration of how secondary themes can selectively reuse NiFi'scomponent partials. Documentation comments explain the three available customization strategies.
Other Notes
.mdc-dialog__contentfont-size intentionally changed from hardcoded14pxtovar(--mat-sys-body-medium-size)to align with the typography scale. Currently resolves to the same value.
hover:cursor-pointeradded directly to copy-button template via Tailwind class, replacing aglobal
.copy-button:hoverrule.Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000VerifiedstatusPull Request Formatting
mainbranchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
./mvnw clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation