Copilot commented on code in PR #8517:
URL: https://github.com/apache/texera/pull/8517#discussion_r3997121870
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -902,19 +925,59 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
* stored per-sub-field overrides (rename, hide), keyed by field path. A
repeated section builds
* its row template on demand, so its builder is wrapped to decorate every
row formly ever makes.
*/
- private applyFieldOverrides(field: FormlyFieldConfig, binding:
FormFieldBinding): void {
+ private applyFieldOverrides(field: FormlyFieldConfig, binding:
FormFieldBinding, schemaLabel: string): void {
const walk = (node: FormlyFieldConfig, path: string): void => {
// Drop the schema's own description on every field, nested ones
included: on this page the
// one piece of guidance is the help text the form's author writes,
rendered once by the card.
node.props = { ...(node.props ?? {}), description: "" };
+ // Author mode, the input itself (root path): its name is renamed in
place by clicking the
+ // title, like every nested field. No eye here -- a whole input leaves
via Remove, not a hide
+ // toggle. The editable label becomes the single title, so formly's own
label is cleared to
+ // avoid printing it twice.
+ if (!path && this.authoring) {
+ EditableLabelWrapperComponent.decorate(
+ node,
+ { authoring: true, name: binding.displayName ?? "", hidden: false,
fallback: schemaLabel, canHide: false },
+ name => this.onBindingNamed(binding.id, name)
+ );
+ node.props = { ...(node.props ?? {}), label: "" };
+ } else if (!path && node.type === "array") {
Review Comment:
`!path` is also true for a leaf row of a root scalar array: both
scalar-array branches call `walk(row, path)` with the root's empty path. In
authoring mode, every scalar row therefore gets another editable wrapper
showing the whole binding title and renaming the whole binding. Restrict this
decoration to the actual root field rather than every node with an empty path.
##########
frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.html:
##########
@@ -0,0 +1,75 @@
+<!--
+ 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.
+-->
+
+<!-- Authoring: the label is the input, so what you type is what the reader
reads. Written through on
+ every keystroke (input, not change): the hidden label below and the saved
name follow the box as
+ it is typed, so nothing is lost if the page is left while the box still
has the focus. -->
+<div
+ class="lbl-row"
+ *ngIf="props.authoring">
+ <input
+ class="lbl-input"
+ [value]="props.authorName"
+ [placeholder]="props.schemaLabel"
+ (input)="onRename($event)"
+ [attr.aria-label]="'Label shown above this input'"
+ [title]="'Shown above this box. Empty keeps ' + props.schemaLabel" />
+ <!-- An input the reader can only lose by removing it altogether has no eye:
offering
+ one here would be a second place to decide the same thing. -->
+ <!-- A toggle button: the name stays constant and aria-pressed carries the
state (a name that
+ changed with the state would announce the opposite of what the state
says). -->
+ <button
+ *ngIf="props.canHide !== false"
+ type="button"
+ class="lbl-eye"
+ [class.off]="props.authorHidden"
+ (click)="onToggleHidden()"
+ [attr.aria-pressed]="props.authorHidden === true"
+ aria-label="Hide from the form"
+ [title]="props.authorHidden ? 'Hidden from the form' : 'Shown on the
form'">
+ <i
+ nz-icon
+ [nzType]="props.authorHidden ? 'eye-invisible' : 'eye'"
+ nzTheme="outline"></i>
+ </button>
+</div>
+
+<!-- The name box above edits the label; it is not the label. The control
below still needs a
+ programmatic name (decorate blanks formly's own), so it gets one that
follows the current name,
+ present for assistive technology only. -->
+<label
+ class="lbl-sr-only"
+ *ngIf="props.authoring"
+ [attr.for]="id">
+ {{ props.authorName || props.schemaLabel }}
+</label>
+
+<!-- Everyone else just reads it. This wrapper blanks formly's own label
(decorate sets label to ""),
+ so this one has to do that label's job in full: `for` ties it to the
control (formly gives the
+ control the field id), or the reader's input would show a name and
announce none. -->
+<label
+ class="lbl-static"
+ *ngIf="!props.authoring && (props.authorName || props.schemaLabel)"
+ [attr.for]="id">
+ {{ props.authorName || props.schemaLabel }}
Review Comment:
This reader-mode wrapper is added specifically for array fields, but
`ArrayTypeComponent` renders rows and buttons without any native labelable
element carrying the parent field `id`. Consequently this `for` points to no
labelable control, so the repeated section title is not programmatically
associated as claimed. Give the repeated section group semantics (for example,
a labelled `role="group"`) instead of relying on `label[for]` for the array
container.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -902,19 +925,59 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
* stored per-sub-field overrides (rename, hide), keyed by field path. A
repeated section builds
* its row template on demand, so its builder is wrapped to decorate every
row formly ever makes.
*/
- private applyFieldOverrides(field: FormlyFieldConfig, binding:
FormFieldBinding): void {
+ private applyFieldOverrides(field: FormlyFieldConfig, binding:
FormFieldBinding, schemaLabel: string): void {
const walk = (node: FormlyFieldConfig, path: string): void => {
// Drop the schema's own description on every field, nested ones
included: on this page the
// one piece of guidance is the help text the form's author writes,
rendered once by the card.
node.props = { ...(node.props ?? {}), description: "" };
+ // Author mode, the input itself (root path): its name is renamed in
place by clicking the
+ // title, like every nested field. No eye here -- a whole input leaves
via Remove, not a hide
+ // toggle. The editable label becomes the single title, so formly's own
label is cleared to
+ // avoid printing it twice.
+ if (!path && this.authoring) {
+ EditableLabelWrapperComponent.decorate(
+ node,
+ { authoring: true, name: binding.displayName ?? "", hidden: false,
fallback: schemaLabel, canHide: false },
+ name => this.onBindingNamed(binding.id, name)
+ );
+ node.props = { ...(node.props ?? {}), label: "" };
+ } else if (!path && node.type === "array") {
+ // Reader mode, a repeated input: the shared array widget prints its
label at the BOTTOM,
+ // beside its add button (the canvas panel's convention), while every
other widget and the
+ // author's editable title sit above. Left alone, the title would jump
from above the rows
+ // in edit mode to below them on Done. Give it the same static title
above instead; the
+ // wrapper blanks the widget's own label.
+ EditableLabelWrapperComponent.decorate(node, {
+ authoring: false,
+ name: binding.displayName ?? "",
+ hidden: false,
+ fallback: schemaLabel,
+ canHide: false,
+ });
+ }
// Apply the author's stored overrides so a reader sees each sub-field
renamed and hidden as
// set up. The root (path "") carries the binding's own displayName, set
in renderField.
if (path) {
const override = binding.overrides?.[path] ?? {};
if (override.displayName) {
node.props = { ...(node.props ?? {}), label: override.displayName };
}
- if (override.hidden) {
+ if (this.authoring) {
+ // An author edits the sub-field's label where it appears and keeps
hidden fields on
+ // screen (faded, via the wrapper) so they can be brought back,
rather than removed from
+ // the DOM as they are for a reader.
+ EditableLabelWrapperComponent.decorate(
+ node,
+ {
+ authoring: true,
+ name: override.displayName ?? "",
+ hidden: override.hidden === true,
+ fallback: (node.props?.label as string) || path,
Review Comment:
For an already-renamed sub-field, line 963 replaces `node.props.label`
before this fallback is read, so `schemaLabel` becomes the old override.
Clearing the editor then continues to display that old custom name instead of
the schema label until a rebuild. Capture the original label before applying
`override.displayName` and pass that captured value here.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -1241,6 +1380,24 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
}
}
+ /**
+ * Renaming the input itself, from its own title. Like the help text, a name
or a hide flag is
+ * presentation only: the wrapper that took the edit already shows it, so
the form is NOT rebuilt
+ * here. A rebuild would replace the very control the author is on (the name
box, the eye) and drop
+ * the keyboard focus with it; the stored override is applied on the next
full re-read (Done).
+ */
+ private onBindingNamed(bindingId: string, value: string): void {
+ this.formBindingService.updateBinding(bindingId, { displayName: value });
+ }
+
+ private onSubFieldNamed(bindingId: string, path: string, value: string):
void {
+ this.formBindingService.setFieldOverride(bindingId, path, { displayName:
value });
+ }
+
+ private onSubFieldHiddenAt(bindingId: string, path: string, hidden:
boolean): void {
+ this.formBindingService.setFieldOverride(bindingId, path, { hidden });
Review Comment:
This write synchronously emits `formBindingChanged$`, whose subscriber at
lines 498–500 immediately rebuilds because the focused eye is a button and
`isTypingInTheForm()` explicitly excludes buttons. That replaces the wrapper
and defeats the stated focus-preservation behavior after hiding/unhiding.
Suppress the local presentation edit's rebuild (while retaining the event for
autosave), or otherwise distinguish it from changes that require rebuilding.
--
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]
