NIFI-15854 - Support asset property types for connectors#11193
NIFI-15854 - Support asset property types for connectors#11193rfellows wants to merge 4 commits intoapache:mainfrom
Conversation
|
Reviewing... |
| .upload-icon { | ||
| color: var(--mat-sys-on-surface-variant); | ||
| font-size: 18px; | ||
| line-height: 1; | ||
| } |
There was a problem hiding this comment.
| .upload-icon { | |
| color: var(--mat-sys-on-surface-variant); | |
| font-size: 18px; | |
| line-height: 1; | |
| } |
| @if (assets.length === 0 && uploadProgress.length === 0) { | ||
| <div class="flex flex-col items-center justify-center gap-3 p-2"> | ||
| <div class="flex items-center gap-2"> | ||
| <i class="fa fa-cloud-upload upload-icon" aria-hidden="true"></i> |
There was a problem hiding this comment.
| <i class="fa fa-cloud-upload upload-icon" aria-hidden="true"></i> | |
| <i class="fa fa-cloud-upload" aria-hidden="true"></i> |
| (invalidDrop)="onInvalidDrop($event)" | ||
| data-qa="asset-drop-zone"> | ||
| @if (assets.length === 0 && uploadProgress.length === 0) { | ||
| <div class="flex flex-col items-center justify-center gap-3 p-2"> |
There was a problem hiding this comment.
| <div class="flex flex-col items-center justify-center gap-3 p-2"> | |
| <div class="flex flex-col items-center justify-center gap-3"> |
| [class.progress-bar-critical]="progress.status === 'error'"> | ||
| </mat-progress-bar> | ||
| @if (progress.status === 'active') { | ||
| <span class="text-sm mr-2 whitespace-nowrap">{{ progress.percentComplete }}%</span> |
There was a problem hiding this comment.
Please check the other mat-progress-bar usages throughout nifi:
| <span class="text-sm mr-2 whitespace-nowrap">{{ progress.percentComplete }}%</span> | |
| <span class="tertiary-color font-medium">{{ progress.percentComplete }}%</span> |
| .progress-item.error { | ||
| border-color: var(--mat-sys-error); | ||
| } | ||
|
|
||
| .mat-mdc-progress-bar.progress-bar-critical { | ||
| --mdc-linear-progress-active-indicator-color: var(--mat-sys-error); | ||
| } |
There was a problem hiding this comment.
Please use mat-progress-bar color input instead.
| .progress-item.error { | |
| border-color: var(--mat-sys-error); | |
| } | |
| .mat-mdc-progress-bar.progress-bar-critical { | |
| --mdc-linear-progress-active-indicator-color: var(--mat-sys-error); | |
| } |
There was a problem hiding this comment.
The issue is that while we do have the mat-progress-bar in our design system toolbox here we do NOT have any success, caution, and error theming.
I was trying to work around this by leveraging the color input but as you said that is a no-op because we are using M3 and it's tokens and the theming mixins etc.
I also see that you attempted to do something similar with a mat-progress-bar in the connector-verification-progress-dialog.component.scss and template.
We need to clean up these styles. It is obvious we have a need to add support for a success, caution, and error state to the mat-progress-bar design system component. Something like:
@use '@angular/material' as mat;
@mixin generate-material-theme() {
:root {
.progress-bar-success {
@include mat.progress-bar-overrides((
active-indicator-color: var(--mat-sys-primary),
));
}
.progress-bar-caution {
@include mat.progress-bar-overrides((
active-indicator-color: var(--nf-caution-default),
));
}
.progress-bar-error {
@include mat.progress-bar-overrides((
active-indicator-color: var(--mat-sys-error),
));
}
}
.darkMode {
.progress-bar-success {
@include mat.progress-bar-overrides((
active-indicator-color: var(--mat-sys-primary),
));
}
.progress-bar-caution {
@include mat.progress-bar-overrides((
active-indicator-color: var(--nf-caution-default),
));
}
.progress-bar-error {
@include mat.progress-bar-overrides((
active-indicator-color: var(--mat-sys-error),
));
}
}
}
somewhere towards the bottom of _app.scss generate-material-theme() mixin.
| .progress-item.error { | ||
| border-color: var(--mat-sys-error); | ||
| } | ||
|
|
||
| .mat-mdc-progress-bar.progress-bar-critical { | ||
| --mdc-linear-progress-active-indicator-color: var(--mat-sys-error); | ||
| } |
There was a problem hiding this comment.
The issue is that while we do have the mat-progress-bar in our design system toolbox here we do NOT have any success, caution, and error theming.
I was trying to work around this by leveraging the color input but as you said that is a no-op because we are using M3 and it's tokens and the theming mixins etc.
I also see that you attempted to do something similar with a mat-progress-bar in the connector-verification-progress-dialog.component.scss and template.
We need to clean up these styles. It is obvious we have a need to add support for a success, caution, and error state to the mat-progress-bar design system component. Something like:
@use '@angular/material' as mat;
@mixin generate-material-theme() {
:root {
.progress-bar-success {
@include mat.progress-bar-overrides((
active-indicator-color: var(--mat-sys-primary),
));
}
.progress-bar-caution {
@include mat.progress-bar-overrides((
active-indicator-color: var(--nf-caution-default),
));
}
.progress-bar-error {
@include mat.progress-bar-overrides((
active-indicator-color: var(--mat-sys-error),
));
}
}
.darkMode {
.progress-bar-success {
@include mat.progress-bar-overrides((
active-indicator-color: var(--mat-sys-primary),
));
}
.progress-bar-caution {
@include mat.progress-bar-overrides((
active-indicator-color: var(--nf-caution-default),
));
}
.progress-bar-error {
@include mat.progress-bar-overrides((
active-indicator-color: var(--mat-sys-error),
));
}
}
}
somewhere towards the bottom of _app.scss generate-material-theme() mixin.
…ng a failed upload by reconciling form-shape unsaved values against API-shape AssetReferences in initializeForm.
scottyaslan
left a comment
There was a problem hiding this comment.
A few more minor things.
| background-color: var(--nf-caution-default); | ||
| } | ||
|
|
||
| /* progress-bar status overrides */ |
There was a problem hiding this comment.
Yes, but could you please move these down? There are some other component overrides towards the bottom for mat-select and mat-option. This belongs there.
| data-qa="property-input-asset-upload"> | ||
| </asset-upload> | ||
| @if (prop.description && (!parentControl?.invalid || !parentControl?.touched)) { | ||
| <mat-hint class="text-xs">{{ prop.description }}</mat-hint> |
There was a problem hiding this comment.
mat-hint should already adhere to the design system
| <mat-hint class="text-xs">{{ prop.description }}</mat-hint> | |
| <mat-hint>{{ prop.description }}</mat-hint> |
| <mat-hint class="text-xs">{{ prop.description }}</mat-hint> | ||
| } | ||
| @if (parentControl?.hasError('required') && parentControl?.touched) { | ||
| <mat-error class="error-color text-xs" data-qa="property-input-asset-required-error"> |
There was a problem hiding this comment.
Aren't mat-error already colored correctly?
| <mat-error class="error-color text-xs" data-qa="property-input-asset-required-error"> | |
| <mat-error data-qa="property-input-asset-required-error"> |
| </mat-error> | ||
| } | ||
| @if (parentControl?.hasError('assetContentMissing') && parentControl?.touched) { | ||
| <mat-error class="error-color text-xs" data-qa="property-input-asset-missing-error"> |
There was a problem hiding this comment.
mat-error should already adhere to the design system
| <mat-error class="error-color text-xs" data-qa="property-input-asset-missing-error"> | |
| <mat-error data-qa="property-input-asset-missing-error"> |
| </mat-error> | ||
| } | ||
| @if (parentControl?.hasError('verificationError')) { | ||
| <mat-error class="error-color text-xs" data-qa="property-input-asset-verification-error"> |
There was a problem hiding this comment.
| <mat-error class="error-color text-xs" data-qa="property-input-asset-verification-error"> | |
| <mat-error data-qa="property-input-asset-verification-error"> |
| <div | ||
| class="asset-item-missing-content caution-color-background flex items-center justify-between gap-3 px-3 py-2 border rounded-md min-h-[44px]" | ||
| data-qa="uploaded-asset-missing-content"> | ||
| <div class="flex items-start gap-2 min-w-0 flex-1"> | ||
| <i | ||
| class="fa fa-warning caution-color shrink-0" |
There was a problem hiding this comment.
The issue here now is that the caution-color-background and the caution-color are defined in _app.scss as:
.caution-color {
color: var(--nf-caution-default);
fill: var(--nf-caution-default);
}
.caution-color-background {
background-color: var(--nf-caution-default);
}
So here the icon will be the same color as the background. Typically in nifi we do not color text or surfaces with really anything outside of what the design system provides. Is it necessary here? Do you need the caution-color-background applied to the container? Typically in our listings when we show the fa-warning icon for an invalid extension for example the icon is caution-colored but the surface is uncustomized.
Summary
NIFI-15854
Connector property descriptors of type
ASSETandASSET_LISTpreviously rendered asplain text inputs in the connector configuration wizard, with no way to upload, browse,
or remove files. Users could not configure connectors that require uploaded assets
(keystores, credentials JSON, certificates, etc.) without manually constructing value
references. This change adds a first-class asset-upload experience to the wizard so
those property types can be configured end-to-end from the UI.
Changes
AssetUploadcomponent (libs/shared/src/components/asset-upload/) — a reusable,reactive-form-friendly file upload control that presents a drop zone, a Browse button,
the list of uploaded assets, in-flight upload progress bars, and a dismiss affordance
for failed uploads. Implements
ControlValueAccessorso it can be bound directly with[formControl]. Supports both single-file and multi-file modes via amultipleinputand accepts optional
allowedFileTypesandmaxFileSizeconstraints.DragAndDropDirective(libs/shared/src/directives/drag-and-drop/) — appliesdrop-allowed/drop-invalidhost classes during dragover, validates dropped filecount, type, and size against the host's configured constraints, and emits
filesDropped(FileList) andinvalidDrop(string) events. File-extension matching iscase-insensitive.
ConnectorPropertyInputnow renders<asset-upload>for property descriptors oftype
ASSET(single) andASSET_LIST(multi). Two predicates (shouldUseAssetUploadand
isMultipleAssets) drive the conditional render, and three forwarder methods(
onAssetFilesSelected,onAssetDeleteRequested,onDismissFailedUpload) re-emit thechild events on the component's existing outputs so the parent
connector-configuration-stepcan drive uploads via the existingUploadServiceandConnectorConfigurationService.createAssetUploadRequest.<mat-error>elements for theexisting
required,assetContentMissing, andverificationErrorvalidators, plus a<mat-hint>for the property description. The description hint is suppressed while atouched validation error is visible to avoid duplicated guidance.
shouldUseTextInput()helper removed fromConnectorPropertyInput.libs/shared/src/components/index.tsandlibs/shared/src/directives/index.ts.Out of scope
This change is purely the wizard UI surface for asset properties. It deliberately does
not touch:
POST /connectors/{id}/assetsHTTP contract — already handled byUploadService.addAsset,removeAsset,currentAssets, andassetUploadProgresswere already in place.ConnectorValueReferenceinvalue-reference.helper.ts.assetContentMissingvalidator inconnector-validation.utils.ts.connector-configuration-step.component.html, whichalready binds the asset-related inputs and outputs.