scottyaslan commented on code in PR #9288: URL: https://github.com/apache/nifi/pull/9288#discussion_r1772425871
########## nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/documentation/ui/common/restrictions-definition/restrictions-definition.component.html: ########## @@ -0,0 +1,43 @@ +<!-- + ~ 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. + --> + +<div class="restrictions-definition informational-table"> + <table mat-table [dataSource]="dataSource"> + <!-- Restrictions --> + <ng-container matColumnDef="required-permission"> + <th mat-header-cell *matHeaderCellDef title="Required Permission"> + <div class="truncate">Required Permission</div> + </th> + <td mat-cell *matCellDef="let item" [title]="item.requiredPermission" class="tertiary-color font-medium"> Review Comment: In this case the issue is that the tables have hover state and the tertiary color does not have enough contrast ration with the hover state. I think we probably want to align with the property table here and use 'font-bold': ```suggestion <td mat-cell *matCellDef="let item" [title]="item.requiredPermission" class="font-medium"> ``` ########## nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/documentation/ui/overview/overview.component.html: ########## @@ -0,0 +1,68 @@ +<!-- + ~ 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. + --> + +<div class="p-4"> + <h3 class="primary-color">Overview</h3> + <div class="flex flex-col gap-y-4"> + <div class="tertiary-color font-medium"> Review Comment: Typically we use the font-medium tertiary-color combination for 'value' text of 'label/value' pairs. In this case I would just allow the base typography. ```suggestion <div> ``` ########## nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/documentation/feature/_documentation.component-theme.scss: ########## @@ -0,0 +1,53 @@ +/* + * 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. + */ + +@use 'sass:map'; +@use '@angular/material' as mat; + +@mixin generate-theme($material-theme, $config) { + $is-material-dark: if(mat.get-theme-type($material-theme) == dark, true, false); + + $material-theme-tertiary-palette-darker: mat.get-theme-color( + $material-theme, + tertiary, + map.get(map.get($config, tertiary), darker) + ); + $material-theme-tertiary-palette-lighter: mat.get-theme-color( + $material-theme, + tertiary, + map.get(map.get($config, tertiary), lighter) + ); + + $selected-item-color: if( + $is-material-dark, + $material-theme-tertiary-palette-darker, + $material-theme-tertiary-palette-lighter + ); + + .documentation { Review Comment: IMO the `h2` in the documentation should use some additional bottom margin: ```suggestion .documentation { h2 { margin-bottom: 12px !important; } ``` ########## nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/documentation/ui/common/attributes-definition/attributes-definition.component.html: ########## @@ -0,0 +1,43 @@ +<!-- + ~ 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. + --> + +<div class="attributes-definition informational-table"> + <table mat-table [dataSource]="dataSource"> + <!-- Name --> + <ng-container matColumnDef="name"> + <th mat-header-cell *matHeaderCellDef title="Name"> + <div class="truncate">Name</div> + </th> + <td mat-cell *matCellDef="let item" [title]="item.name" class="tertiary-color font-medium"> Review Comment: In this case the issue is that the tables have hover state and the tertiary color does not have enough contrast ration with the hover state. I think we probably want to align with the property table here and use 'font-bold': ```suggestion <td mat-cell *matCellDef="let item" [title]="item.name" class="font-bold"> ``` ########## nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/documentation/ui/common/relationships-definition/relationships-definition.component.html: ########## @@ -0,0 +1,43 @@ +<!-- + ~ 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. + --> + +<div class="relationships-definition informational-table"> + <table mat-table [dataSource]="dataSource"> + <!-- Name --> + <ng-container matColumnDef="name"> + <th mat-header-cell *matHeaderCellDef title="Name"> + <div class="truncate">Name</div> + </th> + <td mat-cell *matCellDef="let item" [title]="item.name" class="tertiary-color font-medium"> Review Comment: In this case the issue is that the tables have hover state and the tertiary color does not have enough contrast ration with the hover state. I think we probably want to align with the property table here and use 'font-bold': ```suggestion <td mat-cell *matCellDef="let item" [title]="item.name" class="font-bold"> ``` ########## nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/documentation/ui/overview/overview.component.html: ########## @@ -0,0 +1,68 @@ +<!-- + ~ 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. + --> + +<div class="p-4"> + <h3 class="primary-color">Overview</h3> + <div class="flex flex-col gap-y-4"> + <div class="tertiary-color font-medium"> + Apache NiFi is a dataflow system based on the concepts of flow-based programming. It supports powerful and + scalable directed graphs of data routing, transformation, and system mediation logic. NiFi has a web-based + user interface for design, control, feedback, and monitoring of dataflows. It is highly configurable along + several dimensions of quality of service, such as loss-tolerant versus guaranteed delivery, low latency + versus high throughput, and priority-based queuing. NiFi provides fine-grained data provenance for all data + received, forked, joined cloned, modified, sent, and ultimately dropped upon reaching its configured + end-state. + </div> + <div class="tertiary-color font-medium"> Review Comment: Typically we use the font-medium tertiary-color combination for 'value' text of 'label/value' pairs. In this case I would just allow the base typography. ```suggestion <div> ``` ########## nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/documentation/ui/common/resource-considerations/resource-considerations.component.html: ########## @@ -0,0 +1,43 @@ +<!-- + ~ 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. + --> + +<div class="resource-considerations informational-table"> + <table mat-table [dataSource]="dataSource"> + <!-- Resource --> + <ng-container matColumnDef="resource"> + <th mat-header-cell *matHeaderCellDef title="Resource"> + <div class="truncate">Resource</div> + </th> + <td mat-cell *matCellDef="let item" [title]="item.resource" class="tertiary-color font-medium"> Review Comment: In this case the issue is that the tables have hover state and the tertiary color does not have enough contrast ration with the hover state. I think we probably want to align with the property table here and use 'font-bold': ```suggestion <td mat-cell *matCellDef="let item" [title]="item.resource" class="font-bold"> ``` ########## nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/documentation/ui/common/configurable-extension-definition/configurable-extension-definition.component.html: ########## @@ -0,0 +1,115 @@ +<!-- + ~ 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. + --> + +@if (configurableExtensionDefinition) { + <h3 class="primary-color"> + {{ formatExtensionName(configurableExtensionDefinition.type) }} {{ configurableExtensionDefinition.version }} + </h3> + <div class="flex flex-col gap-y-4"> + @if (configurableExtensionDefinition.deprecated) { + <div class="caution-color-background p-4 flex flex-col gap-y-2"> + @if (configurableExtensionDefinition.deprecationReason) { + <div>{{ configurableExtensionDefinition.deprecationReason }}</div> + } @else { + <div>Please be aware this processor is deprecated and may be removed in the near future.</div> + } + @if (configurableExtensionDefinition.deprecationAlternatives) { + <div> + <div>See Alternatives</div> + <see-also [extensionTypes]="configurableExtensionDefinition.deprecationAlternatives"></see-also> + </div> + } + </div> + } + <div> + <div>Bundle</div> + <div class="tertiary-color font-medium"> + {{ configurableExtensionDefinition.group }} | {{ configurableExtensionDefinition.artifact }} + </div> + </div> + @if (configurableExtensionDefinition.typeDescription) { + <div> + <div>Description</div> + <div class="tertiary-color font-medium">{{ configurableExtensionDefinition.typeDescription }}</div> + </div> + } + @if (configurableExtensionDefinition.additionalDetails) { + <additional-details + [configurableExtensionDefinition]="configurableExtensionDefinition"></additional-details> + } + <div> + <div>Tags</div> + <div class="tertiary-color font-medium">{{ configurableExtensionDefinition.tags.join(', ') }}</div> + </div> + @if (configurableExtensionDefinition.propertyDescriptors) { + <div> + <h2>Properties</h2> + <properties-definition + [configurableExtensionDefinition]="configurableExtensionDefinition"></properties-definition> + </div> + } + @if (configurableExtensionDefinition.dynamicProperties) { + <div> + <h2>Dynamic Properties</h2> + <dynamic-properties-definition + [configurableExtensionDefinition]="configurableExtensionDefinition"></dynamic-properties-definition> + </div> + } + @if (configurableExtensionDefinition.stateful) { + <div> + <h2>State Management</h2> + <div> + <div class="flex flex-col gap-y-1"> Review Comment: ```suggestion <div class="flex flex-col gap-y-4"> ``` -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
