You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "scottyaslan (via GitHub)" <gi...@apache.org> on 2026/05/11 21:46:30 UTC
[PR] NIFI-15927 improvements to connector property group card [nifi]
scottyaslan opened a new pull request, #11237:
URL: https://github.com/apache/nifi/pull/11237
<!-- Licensed to the Apache Software Foundation (ASF) under one or more -->
<!-- contributor license agreements. See the NOTICE file distributed with -->
<!-- this work for additional information regarding copyright ownership. -->
<!-- The ASF licenses this file to You under the Apache License, Version 2.0 -->
<!-- (the "License"); you may not use this file except in compliance with -->
<!-- the License. You may obtain a copy of the License at -->
<!-- http://www.apache.org/licenses/LICENSE-2.0 -->
<!-- Unless required by applicable law or agreed to in writing, software -->
<!-- distributed under the License is distributed on an "AS IS" BASIS, -->
<!-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -->
<!-- See the License for the specific language governing permissions and -->
<!-- limitations under the License. -->
# Summary
<img width="1427" height="927" alt="Screenshot 2026-05-11 at 5 43 28 PM" src="https://github.com/user-attachments/assets/8251f613-bcfc-4630-9908-d31036c65f99" />
<img width="1425" height="930" alt="Screenshot 2026-05-11 at 5 43 51 PM" src="https://github.com/user-attachments/assets/85b599db-3b98-4124-a146-8ce86a8b512d" />
[NIFI-15927](https://issues.apache.org/jira/browse/NIFI-15927)
# Tracking
Please complete the following tracking steps prior to pull request creation.
### Issue Tracking
- [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
### Pull Request Tracking
- Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-00000`
- Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-00000`
- Pull request contains [commits signed](https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits) with a registered key indicating `Verified` status
### Pull Request Formatting
- Pull Request based on current revision of the `main` branch
- Pull Request refers to a feature branch with one commit containing changes
# Verification
Please indicate the verification steps performed prior to pull request creation.
### Build
- [ ] Build completed using `./mvnw clean install -P contrib-check`
- [ ] JDK 21
- [ ] JDK 25
### Licensing
- [ ] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)
- [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
### Documentation
- [ ] Documentation formatting appears as expected in rendered files
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-15927 improvements to connector property group card [nifi]
Posted by "scottyaslan (via GitHub)" <gi...@apache.org>.
scottyaslan commented on code in PR #11237:
URL: https://github.com/apache/nifi/pull/11237#discussion_r3229029551
##########
nifi-frontend/src/main/frontend/libs/shared/src/components/property-group-card/property-group-card.component.html:
##########
@@ -0,0 +1,44 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+<mat-card appearance="outlined" class="p-4">
+ @if (!hideGroupName()) {
+ <div class="font-semibold mb-4">{{ propertyGroup().propertyGroupName }}</div>
+ }
+ <div class="grid grid-cols-1 sm:grid-cols-2 lg:grid-cols-3 xl:grid-cols-4 gap-x-6 gap-y-4">
+ @for (propertyName of getPropertyNames(); track propertyName) {
+ <div class="min-w-0 flex flex-col gap-y-1">
+ <div class="flex items-center pt-1">
+ <mat-label ellipsisTooltip>{{ propertyName }}</mat-label>
Review Comment:
Good catch. Added MatLabel to imports.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-15927 improvements to connector property group card [nifi]
Posted by "mcgilman (via GitHub)" <gi...@apache.org>.
mcgilman commented on code in PR #11237:
URL: https://github.com/apache/nifi/pull/11237#discussion_r3237710351
##########
nifi-frontend/src/main/frontend/libs/shared/src/assets/styles/_app.scss:
##########
@@ -306,20 +306,6 @@
line-height: normal;
}
- .mat-mdc-card-title {
- font-size: 18px;
- letter-spacing: normal;
- line-height: 28px;
- font-weight: 700;
- }
-
- .mat-mdc-card-subtitle {
Review Comment:
We're removing global styles here. Jolt UI uses `<mat-card-subtitle>`. Have we verified these changes everywhere?
##########
nifi-frontend/src/main/frontend/libs/shared/src/assets/styles/_app.scss:
##########
@@ -799,4 +785,13 @@
pointer-events: auto;
cursor: not-allowed;
}
+
+ // mat-card
+ @include mat.card-overrides(
+ (
+ title-text-size: 16px,
+ title-text-line-height: 20px,
+ title-text-weight: 600,
Review Comment:
My comment previously was that in that instance the card title was too big. The change would affect all card titles throughout the application. Is this appropriate?
##########
nifi-frontend/src/main/frontend/libs/shared/src/components/property-group-card/property-group-card.component.spec.ts:
##########
@@ -0,0 +1,376 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { TestBed } from '@angular/core/testing';
+import { By } from '@angular/platform-browser';
+import { NoopAnimationsModule } from '@angular/platform-browser/animations';
+import { PropertyGroupCard } from './property-group-card.component';
+import { ConfigVerificationResult, PropertyGroupConfiguration } from '../../types';
+
+describe('PropertyGroupCard', () => {
+ function makeGroup(overrides: Partial<PropertyGroupConfiguration> = {}): PropertyGroupConfiguration {
+ return {
+ propertyGroupName: 'Connection',
+ propertyDescriptors: {
+ Host: { name: 'Host', type: 'STRING', required: true },
+ Port: { name: 'Port', type: 'STRING', required: false }
+ },
+ propertyValues: {
+ Host: { value: 'localhost', valueType: 'STRING_LITERAL' },
+ Port: { value: '5432', valueType: 'STRING_LITERAL' }
+ },
+ ...overrides
+ };
+ }
+
+ interface SetupOptions {
+ propertyGroup?: PropertyGroupConfiguration;
+ hideGroupName?: boolean;
+ verificationErrors?: ConfigVerificationResult[];
+ }
+
+ async function setup(options: SetupOptions = {}) {
+ await TestBed.configureTestingModule({
+ imports: [PropertyGroupCard, NoopAnimationsModule]
+ }).compileComponents();
+
+ const fixture = TestBed.createComponent(PropertyGroupCard);
+ const component = fixture.componentInstance;
+
+ fixture.componentRef.setInput('propertyGroup', options.propertyGroup ?? makeGroup());
+ if (options.hideGroupName !== undefined) fixture.componentRef.setInput('hideGroupName', options.hideGroupName);
+ if (options.verificationErrors !== undefined)
+ fixture.componentRef.setInput('verificationErrors', options.verificationErrors);
+
+ fixture.detectChanges();
+
+ const queryAll = (qa: string) => fixture.debugElement.queryAll(By.css(`[data-qa="${qa}"]`));
+ const query = (qa: string) => fixture.debugElement.query(By.css(qa));
Review Comment:
Make both helpers handle the same kind of argument (e.g., both take a data-qa value, or both take a raw selector). Or maybe rename `qa` to selector in the `query` case.
##########
nifi-frontend/src/main/frontend/libs/shared/src/components/property-group-card/property-group-card.component.spec.ts:
##########
@@ -0,0 +1,376 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { TestBed } from '@angular/core/testing';
+import { By } from '@angular/platform-browser';
+import { NoopAnimationsModule } from '@angular/platform-browser/animations';
+import { PropertyGroupCard } from './property-group-card.component';
+import { ConfigVerificationResult, PropertyGroupConfiguration } from '../../types';
+
+describe('PropertyGroupCard', () => {
+ function makeGroup(overrides: Partial<PropertyGroupConfiguration> = {}): PropertyGroupConfiguration {
+ return {
+ propertyGroupName: 'Connection',
+ propertyDescriptors: {
+ Host: { name: 'Host', type: 'STRING', required: true },
+ Port: { name: 'Port', type: 'STRING', required: false }
+ },
+ propertyValues: {
+ Host: { value: 'localhost', valueType: 'STRING_LITERAL' },
+ Port: { value: '5432', valueType: 'STRING_LITERAL' }
+ },
+ ...overrides
+ };
+ }
+
+ interface SetupOptions {
+ propertyGroup?: PropertyGroupConfiguration;
+ hideGroupName?: boolean;
+ verificationErrors?: ConfigVerificationResult[];
+ }
+
+ async function setup(options: SetupOptions = {}) {
+ await TestBed.configureTestingModule({
+ imports: [PropertyGroupCard, NoopAnimationsModule]
+ }).compileComponents();
+
+ const fixture = TestBed.createComponent(PropertyGroupCard);
+ const component = fixture.componentInstance;
+
+ fixture.componentRef.setInput('propertyGroup', options.propertyGroup ?? makeGroup());
+ if (options.hideGroupName !== undefined) fixture.componentRef.setInput('hideGroupName', options.hideGroupName);
+ if (options.verificationErrors !== undefined)
+ fixture.componentRef.setInput('verificationErrors', options.verificationErrors);
+
+ fixture.detectChanges();
+
+ const queryAll = (qa: string) => fixture.debugElement.queryAll(By.css(`[data-qa="${qa}"]`));
+ const query = (qa: string) => fixture.debugElement.query(By.css(qa));
+
+ return { fixture, component, queryAll, query };
+ }
+
+ describe('creation', () => {
+ it('should create', async () => {
+ const { component } = await setup();
+ expect(component).toBeTruthy();
+ });
+ });
+
+ describe('group name header', () => {
+ it('should display the group name by default', async () => {
+ const { query } = await setup();
+ const title = query('mat-card-title');
+ expect(title).toBeTruthy();
+ expect(title.nativeElement.textContent.trim()).toBe('Connection');
+ });
+
+ it('should hide the group name when hideGroupName is true', async () => {
+ const { query } = await setup({ hideGroupName: true });
+ expect(query('mat-card-title')).toBeFalsy();
+ });
+ });
+
+ describe('getPropertyNames', () => {
+ it('should return all property names from descriptors', async () => {
+ const { component } = await setup();
+ expect(component.getPropertyNames()).toEqual(['Host', 'Port']);
+ });
+
+ it('should return empty array when no descriptors exist', async () => {
+ const { component } = await setup({
+ propertyGroup: makeGroup({ propertyDescriptors: {} })
+ });
+ expect(component.getPropertyNames()).toEqual([]);
+ });
+ });
+
+ describe('getDescriptor', () => {
+ it('should return the descriptor for a known property', async () => {
+ const { component } = await setup();
+ const descriptor = component.getDescriptor('Host');
+ expect(descriptor).toBeDefined();
+ expect(descriptor!.name).toBe('Host');
+ expect(descriptor!.required).toBe(true);
+ });
+
+ it('should return undefined for an unknown property', async () => {
+ const { component } = await setup();
+ expect(component.getDescriptor('NonExistent')).toBeUndefined();
+ });
+ });
+
+ describe('isRequired', () => {
+ it('should return true for required properties', async () => {
+ const { component } = await setup();
+ expect(component.isRequired('Host')).toBe(true);
+ });
+
+ it('should return false for optional properties', async () => {
+ const { component } = await setup();
+ expect(component.isRequired('Port')).toBe(false);
+ });
+
+ it('should return false for unknown properties', async () => {
+ const { component } = await setup();
+ expect(component.isRequired('NonExistent')).toBe(false);
+ });
+ });
+
+ describe('hasValue', () => {
+ it('should return true for STRING_LITERAL with a value', async () => {
+ const { component } = await setup();
+ expect(component.hasValue('Host')).toBe(true);
+ });
+
+ it('should return false when no value reference exists', async () => {
+ const { component } = await setup({
+ propertyGroup: makeGroup({
+ propertyValues: {}
+ })
+ });
+ expect(component.hasValue('Host')).toBe(false);
+ });
+
+ it('should return false for STRING_LITERAL with null value', async () => {
+ const { component } = await setup({
+ propertyGroup: makeGroup({
+ propertyValues: {
+ Host: { value: null, valueType: 'STRING_LITERAL' }
+ }
+ })
+ });
+ expect(component.hasValue('Host')).toBe(false);
+ });
+
+ it('should return false for STRING_LITERAL with empty string', async () => {
+ const { component } = await setup({
+ propertyGroup: makeGroup({
+ propertyValues: {
+ Host: { value: '', valueType: 'STRING_LITERAL' }
+ }
+ })
+ });
+ expect(component.hasValue('Host')).toBe(false);
+ });
+
+ it('should return true for SECRET_REFERENCE regardless of value', async () => {
+ const { component } = await setup({
+ propertyGroup: makeGroup({
+ propertyValues: {
+ Host: { valueType: 'SECRET_REFERENCE' }
+ }
+ })
+ });
+ expect(component.hasValue('Host')).toBe(true);
+ });
+
+ it('should return true for ASSET_REFERENCE with entries', async () => {
+ const { component } = await setup({
+ propertyGroup: makeGroup({
+ propertyValues: {
+ Host: {
+ valueType: 'ASSET_REFERENCE',
+ assetReferences: [{ id: 'asset-1', name: 'cert.pem' }]
+ }
+ }
+ })
+ });
+ expect(component.hasValue('Host')).toBe(true);
+ });
+
+ it('should return false for ASSET_REFERENCE with empty array', async () => {
+ const { component } = await setup({
+ propertyGroup: makeGroup({
+ propertyValues: {
+ Host: { valueType: 'ASSET_REFERENCE', assetReferences: [] }
+ }
+ })
+ });
+ expect(component.hasValue('Host')).toBe(false);
+ });
+ });
+
+ describe('getDisplayValueForProperty', () => {
+ it('should return the string value for STRING_LITERAL', async () => {
+ const { component } = await setup();
+ expect(component.getDisplayValueForProperty('Host')).toBe('localhost');
+ });
+
+ it('should return masked text for SECRET_REFERENCE', async () => {
+ const { component } = await setup({
+ propertyGroup: makeGroup({
+ propertyValues: {
+ Host: { valueType: 'SECRET_REFERENCE' }
+ }
+ })
+ });
+ expect(component.getDisplayValueForProperty('Host')).toBe('••••••••');
+ });
+
+ it('should return comma-separated asset names for ASSET_REFERENCE', async () => {
+ const { component } = await setup({
+ propertyGroup: makeGroup({
+ propertyValues: {
+ Host: {
+ valueType: 'ASSET_REFERENCE',
+ assetReferences: [
+ { id: 'a1', name: 'cert.pem' },
+ { id: 'a2', name: 'key.pem' }
+ ]
+ }
+ }
+ })
+ });
+ expect(component.getDisplayValueForProperty('Host')).toBe('cert.pem, key.pem');
+ });
+
+ it('should fall back to asset id when name is missing', async () => {
+ const { component } = await setup({
+ propertyGroup: makeGroup({
+ propertyValues: {
+ Host: {
+ valueType: 'ASSET_REFERENCE',
+ assetReferences: [{ id: 'a1' }]
+ }
+ }
+ })
+ });
+ expect(component.getDisplayValueForProperty('Host')).toBe('a1');
+ });
+
+ it('should return empty string when no value reference exists', async () => {
+ const { component } = await setup({
+ propertyGroup: makeGroup({ propertyValues: {} })
+ });
+ expect(component.getDisplayValueForProperty('Host')).toBe('');
+ });
+ });
+
+ describe('fieldErrors computed signal', () => {
+ it('should return empty object when no verification errors', async () => {
+ const { component } = await setup();
+ expect(component.fieldErrors()).toEqual({});
+ });
+
+ it('should map errors to matching property names', async () => {
+ const { component } = await setup({
+ verificationErrors: [
+ { outcome: 'FAILED', verificationStepName: 'Check', subject: 'Host', explanation: 'Invalid host' }
+ ]
+ });
+ expect(component.fieldErrors()).toEqual({ Host: 'Invalid host' });
+ });
+
+ it('should ignore errors whose subject is not in the property group', async () => {
+ const { component } = await setup({
+ verificationErrors: [
+ {
+ outcome: 'FAILED',
+ verificationStepName: 'Check',
+ subject: 'UnknownProp',
+ explanation: 'Bad value'
+ }
+ ]
+ });
+ expect(component.fieldErrors()).toEqual({});
+ });
+
+ it('should ignore errors without a subject', async () => {
+ const { component } = await setup({
+ verificationErrors: [
+ { outcome: 'FAILED', verificationStepName: 'Check', explanation: 'General failure' }
+ ]
+ });
+ expect(component.fieldErrors()).toEqual({});
+ });
+ });
+
+ describe('getFieldError', () => {
+ it('should return the error message for a property with a field error', async () => {
+ const { component } = await setup({
+ verificationErrors: [
+ {
+ outcome: 'FAILED',
+ verificationStepName: 'Check',
+ subject: 'Port',
+ explanation: 'Port out of range'
+ }
+ ]
+ });
+ expect(component.getFieldError('Port')).toBe('Port out of range');
+ });
+
+ it('should return null for a property without a field error', async () => {
+ const { component } = await setup();
+ expect(component.getFieldError('Host')).toBeNull();
+ });
+ });
+
+ describe('template rendering', () => {
+ it('should render a row for each property', async () => {
+ const { query } = await setup();
+ const labels = query('mat-card-content').queryAll(By.css('mat-label'));
+ expect(labels.length).toBe(2);
+ const texts = labels.map((el) => el.nativeElement.textContent.trim());
+ expect(texts).toContain('Host');
+ expect(texts).toContain('Port');
+ });
+
+ it('should show required asterisk for required properties', async () => {
+ const { query } = await setup();
+ const content = query('mat-card-content');
+ const asterisks = content.queryAll(By.css('.error-color'));
Review Comment:
This will overmatch. There are multiple elements with this classname.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-15927 improvements to connector property group card [nifi]
Posted by "mcgilman (via GitHub)" <gi...@apache.org>.
mcgilman commented on PR #11237:
URL: https://github.com/apache/nifi/pull/11237#issuecomment-4432776634
Reviewing...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-15927 improvements to connector property group card [nifi]
Posted by "mcgilman (via GitHub)" <gi...@apache.org>.
mcgilman commented on code in PR #11237:
URL: https://github.com/apache/nifi/pull/11237#discussion_r3243239540
##########
nifi-frontend/src/main/frontend/libs/shared/src/assets/styles/_app.scss:
##########
@@ -799,4 +785,16 @@
pointer-events: auto;
cursor: not-allowed;
}
+
+ // mat-card
+ @include mat.card-overrides(
+ (
+ title-text-size: 16px,
+ title-text-line-height: 20px,
+ title-text-weight: 600,
+ subtitle-text-size: 14px,
+ subtitle-text-line-height: 20px,
+ subtitle-text-tracking: normal,
Review Comment:
Please run prettier to remove this trailing comma. Also, do these overrides need `subtitle-text-weight: 400`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-15927 improvements to connector property group card [nifi]
Posted by "mcgilman (via GitHub)" <gi...@apache.org>.
mcgilman commented on code in PR #11237:
URL: https://github.com/apache/nifi/pull/11237#discussion_r3237735702
##########
nifi-frontend/src/main/frontend/libs/shared/src/assets/styles/_app.scss:
##########
@@ -799,4 +785,13 @@
pointer-events: auto;
cursor: not-allowed;
}
+
+ // mat-card
+ @include mat.card-overrides(
+ (
+ title-text-size: 16px,
+ title-text-line-height: 20px,
+ title-text-weight: 600,
Review Comment:
My comment previously was that in that instance the card title was too big. The change would affect all card titles throughout all applications. Is this appropriate?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-15927 improvements to connector property group card [nifi]
Posted by "scottyaslan (via GitHub)" <gi...@apache.org>.
scottyaslan commented on code in PR #11237:
URL: https://github.com/apache/nifi/pull/11237#discussion_r3239041970
##########
nifi-frontend/src/main/frontend/libs/shared/src/assets/styles/_app.scss:
##########
@@ -799,4 +785,13 @@
pointer-events: auto;
cursor: not-allowed;
}
+
+ // mat-card
+ @include mat.card-overrides(
+ (
+ title-text-size: 16px,
+ title-text-line-height: 20px,
+ title-text-weight: 600,
Review Comment:
I think this is ok. We mainly just reduced the title by 2px and adjusted its line height to 20px. Here is the jolt UI wilth these changes:
<img width="1428" height="934" alt="Screenshot 2026-05-14 at 12 15 44 AM" src="https://github.com/user-attachments/assets/969bc48d-014b-49f0-b63f-9920d15acf2f" />
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-15927 improvements to connector property group card [nifi]
Posted by "scottyaslan (via GitHub)" <gi...@apache.org>.
scottyaslan commented on code in PR #11237:
URL: https://github.com/apache/nifi/pull/11237#discussion_r3239037438
##########
nifi-frontend/src/main/frontend/libs/shared/src/assets/styles/_app.scss:
##########
@@ -306,20 +306,6 @@
line-height: normal;
}
- .mat-mdc-card-title {
- font-size: 18px;
- letter-spacing: normal;
- line-height: 28px;
- font-weight: 700;
- }
-
- .mat-mdc-card-subtitle {
Review Comment:
oops forget those. Added.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-15927 improvements to connector property group card [nifi]
Posted by "mcgilman (via GitHub)" <gi...@apache.org>.
mcgilman merged PR #11237:
URL: https://github.com/apache/nifi/pull/11237
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-15927 improvements to connector property group card [nifi]
Posted by "mcgilman (via GitHub)" <gi...@apache.org>.
mcgilman commented on code in PR #11237:
URL: https://github.com/apache/nifi/pull/11237#discussion_r3228304783
##########
nifi-frontend/src/main/frontend/libs/shared/src/components/property-group-card/property-group-card.component.ts:
##########
@@ -31,35 +32,8 @@ import {
*/
@Component({
selector: 'property-group-card',
- standalone: true,
- imports: [MatCard, MatError],
- template: `
- <mat-card appearance="outlined" class="p-4">
- @if (!hideGroupName()) {
- <h4 class="font-semibold mb-4">{{ propertyGroup().propertyGroupName }}</h4>
- }
- <div class="grid grid-cols-1 sm:grid-cols-2 lg:grid-cols-3 xl:grid-cols-4 gap-x-6 gap-y-4">
- @for (propertyName of getPropertyNames(); track propertyName) {
- <div class="flex flex-col gap-y-1">
- <div class="flex items-center pt-1">
- <span class="text-sm tertiary-color leading-5">{{ propertyName }}</span>
- @if (isRequired(propertyName)) {
- <span class="error-color ml-1 text-sm">*</span>
- }
- </div>
- @if (hasValue(propertyName)) {
- <span class="text-sm">{{ getDisplayValueForProperty(propertyName) }}</span>
- } @else {
- <span class="unset neutral-color text-sm">No value set</span>
- }
- @if (getFieldError(propertyName); as errorMessage) {
- <mat-error class="error-color text-xs">{{ errorMessage }}</mat-error>
- }
- </div>
- }
- </div>
- </mat-card>
- `
+ imports: [MatCard, MatCardHeader, MatCardTitle, MatError, EllipsisTooltipDirective],
Review Comment:
`MatCardHeader` and `MatCardTitle` are imported but never used in the template.
##########
nifi-frontend/src/main/frontend/libs/shared/src/components/property-group-card/property-group-card.component.html:
##########
@@ -0,0 +1,44 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+<mat-card appearance="outlined" class="p-4">
+ @if (!hideGroupName()) {
+ <div class="font-semibold mb-4">{{ propertyGroup().propertyGroupName }}</div>
Review Comment:
Is `h4` more appropriate here?
##########
nifi-frontend/src/main/frontend/libs/shared/src/components/property-group-card/property-group-card.component.ts:
##########
@@ -31,35 +32,8 @@ import {
*/
@Component({
selector: 'property-group-card',
- standalone: true,
- imports: [MatCard, MatError],
- template: `
- <mat-card appearance="outlined" class="p-4">
- @if (!hideGroupName()) {
- <h4 class="font-semibold mb-4">{{ propertyGroup().propertyGroupName }}</h4>
- }
- <div class="grid grid-cols-1 sm:grid-cols-2 lg:grid-cols-3 xl:grid-cols-4 gap-x-6 gap-y-4">
- @for (propertyName of getPropertyNames(); track propertyName) {
- <div class="flex flex-col gap-y-1">
- <div class="flex items-center pt-1">
- <span class="text-sm tertiary-color leading-5">{{ propertyName }}</span>
- @if (isRequired(propertyName)) {
- <span class="error-color ml-1 text-sm">*</span>
- }
- </div>
- @if (hasValue(propertyName)) {
- <span class="text-sm">{{ getDisplayValueForProperty(propertyName) }}</span>
- } @else {
- <span class="unset neutral-color text-sm">No value set</span>
- }
- @if (getFieldError(propertyName); as errorMessage) {
- <mat-error class="error-color text-xs">{{ errorMessage }}</mat-error>
- }
- </div>
- }
- </div>
- </mat-card>
- `
+ imports: [MatCard, MatCardHeader, MatCardTitle, MatError, EllipsisTooltipDirective],
+ templateUrl: './property-group-card.component.html'
})
export class PropertyGroupCard {
Review Comment:
Not an issue with this PR specifically but it would be nice to have some test coverage for this component.
##########
nifi-frontend/src/main/frontend/libs/shared/src/components/property-group-card/property-group-card.component.html:
##########
@@ -0,0 +1,44 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+<mat-card appearance="outlined" class="p-4">
+ @if (!hideGroupName()) {
+ <div class="font-semibold mb-4">{{ propertyGroup().propertyGroupName }}</div>
+ }
+ <div class="grid grid-cols-1 sm:grid-cols-2 lg:grid-cols-3 xl:grid-cols-4 gap-x-6 gap-y-4">
+ @for (propertyName of getPropertyNames(); track propertyName) {
+ <div class="min-w-0 flex flex-col gap-y-1">
+ <div class="flex items-center pt-1">
+ <mat-label ellipsisTooltip>{{ propertyName }}</mat-label>
Review Comment:
`<mat-label>` is used outside a `<mat-form-field>` and `MatLabel` isn't in the imports array.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-15927 improvements to connector property group card [nifi]
Posted by "scottyaslan (via GitHub)" <gi...@apache.org>.
scottyaslan commented on code in PR #11237:
URL: https://github.com/apache/nifi/pull/11237#discussion_r3239037438
##########
nifi-frontend/src/main/frontend/libs/shared/src/assets/styles/_app.scss:
##########
@@ -306,20 +306,6 @@
line-height: normal;
}
- .mat-mdc-card-title {
- font-size: 18px;
- letter-spacing: normal;
- line-height: 28px;
- font-weight: 700;
- }
-
- .mat-mdc-card-subtitle {
Review Comment:
oops forget those. I have restored the subtitle overrides. We didn't remove global styles we just moved them and updated the way we override them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-15927 improvements to connector property group card [nifi]
Posted by "scottyaslan (via GitHub)" <gi...@apache.org>.
scottyaslan commented on code in PR #11237:
URL: https://github.com/apache/nifi/pull/11237#discussion_r3228963329
##########
nifi-frontend/src/main/frontend/libs/shared/src/components/property-group-card/property-group-card.component.html:
##########
@@ -0,0 +1,44 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+<mat-card appearance="outlined" class="p-4">
+ @if (!hideGroupName()) {
+ <div class="font-semibold mb-4">{{ propertyGroup().propertyGroupName }}</div>
Review Comment:
Actually, I think:
```
<mat-card-header>
<mat-card-title data-qa="property-group-name">
{{ propertyGroup().propertyGroupName }}
</mat-card-title>
</mat-card-header>
```
would be more appropriate. I will get that updated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-15927 improvements to connector property group card [nifi]
Posted by "mcgilman (via GitHub)" <gi...@apache.org>.
mcgilman commented on code in PR #11237:
URL: https://github.com/apache/nifi/pull/11237#discussion_r3236187936
##########
nifi-frontend/src/main/frontend/libs/shared/src/components/property-group-card/property-group-card.component.html:
##########
@@ -0,0 +1,50 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+<mat-card appearance="outlined">
+ @if (!hideGroupName()) {
+ <mat-card-header>
+ <mat-card-title data-qa="property-group-name">
+ {{ propertyGroup().propertyGroupName }}
+ </mat-card-title>
+ </mat-card-header>
Review Comment:
<img width="824" height="232" alt="Image" src="https://github.com/user-attachments/assets/6e63915b-97bf-43b2-8763-678b2259301b" />
It looks like the card title is the same typography as the header in the summary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-15927 improvements to connector property group card [nifi]
Posted by "scottyaslan (via GitHub)" <gi...@apache.org>.
scottyaslan commented on code in PR #11237:
URL: https://github.com/apache/nifi/pull/11237#discussion_r3229180061
##########
nifi-frontend/src/main/frontend/libs/shared/src/components/property-group-card/property-group-card.component.ts:
##########
@@ -31,35 +32,8 @@ import {
*/
@Component({
selector: 'property-group-card',
- standalone: true,
- imports: [MatCard, MatError],
- template: `
- <mat-card appearance="outlined" class="p-4">
- @if (!hideGroupName()) {
- <h4 class="font-semibold mb-4">{{ propertyGroup().propertyGroupName }}</h4>
- }
- <div class="grid grid-cols-1 sm:grid-cols-2 lg:grid-cols-3 xl:grid-cols-4 gap-x-6 gap-y-4">
- @for (propertyName of getPropertyNames(); track propertyName) {
- <div class="flex flex-col gap-y-1">
- <div class="flex items-center pt-1">
- <span class="text-sm tertiary-color leading-5">{{ propertyName }}</span>
- @if (isRequired(propertyName)) {
- <span class="error-color ml-1 text-sm">*</span>
- }
- </div>
- @if (hasValue(propertyName)) {
- <span class="text-sm">{{ getDisplayValueForProperty(propertyName) }}</span>
- } @else {
- <span class="unset neutral-color text-sm">No value set</span>
- }
- @if (getFieldError(propertyName); as errorMessage) {
- <mat-error class="error-color text-xs">{{ errorMessage }}</mat-error>
- }
- </div>
- }
- </div>
- </mat-card>
- `
+ imports: [MatCard, MatCardHeader, MatCardTitle, MatError, EllipsisTooltipDirective],
+ templateUrl: './property-group-card.component.html'
})
export class PropertyGroupCard {
Review Comment:
added
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-15927 improvements to connector property group card [nifi]
Posted by "scottyaslan (via GitHub)" <gi...@apache.org>.
scottyaslan commented on code in PR #11237:
URL: https://github.com/apache/nifi/pull/11237#discussion_r3239041970
##########
nifi-frontend/src/main/frontend/libs/shared/src/assets/styles/_app.scss:
##########
@@ -799,4 +785,13 @@
pointer-events: auto;
cursor: not-allowed;
}
+
+ // mat-card
+ @include mat.card-overrides(
+ (
+ title-text-size: 16px,
+ title-text-line-height: 20px,
+ title-text-weight: 600,
Review Comment:
I think this is ok. We mainly just reduced the title by 2px and adjusted its line height to 20px. The change I made here is to correctly leverage the angular material mixins to do so. Here is the jolt UI wilth these changes:
<img width="1428" height="934" alt="Screenshot 2026-05-14 at 12 15 44 AM" src="https://github.com/user-attachments/assets/969bc48d-014b-49f0-b63f-9920d15acf2f" />
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org