mengw15 commented on code in PR #8517:
URL: https://github.com/apache/texera/pull/8517#discussion_r3997150754
##########
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 {
Review Comment:
Every presentation write here lands in `setFormBinding`, which emits
`formBindingChanged$`, and this page rebuilds on that emission
(`rebuildFormOrDefer`). Typing is saved by the typing hold, but the eye is a
button: its click triggers a synchronous full rebuild inside the click — the
wrapper's locally shown state never paints, every card's widgets remount, and
the keyboard focus on the just-clicked eye is dropped, the very thing the
wrapper's self-reflection (and its spec) is there to prevent. A value typed
into another field within its write debounce is reverted by the same rebuild.
The page specs cannot see this: the harness's form-binding mock never emits, so
the chain is severed there. One direction, not prescribing: mark the emission
as self-reflected around these sinks and treat it like the typing hold, keeping
the immediate rebuild for the structural writes (expose/remove/reorder).
##########
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 a sub-field that already carries a name override, `node.props.label` was
overwritten with that override a few lines above, so this fallback is the
override, not the schema label. The box's placeholder and its "Empty keeps …"
tooltip then promise the old override — but clearing the box deletes the
override (`setFieldOverride` drops empty names), and the reader gets the schema
label back. Capturing the label before the override is applied and using that
as the fallback makes the promise true.
##########
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,
+ },
+ name => this.onSubFieldNamed(binding.id, path, name),
+ hidden => this.onSubFieldHiddenAt(binding.id, path, hidden)
Review Comment:
Inside a repeated section this decorate runs on every row's leaves with the
same template path: with three KeyValue pairs the author sees three copies of
the same name box and eye per column, all writing one override, and the copies
do not follow each other's edits (each wrapper shows only its own props; today
they converge only through the full rebuild of the thread above, so fixing that
alone would leave them stale until Done). A scalar-row array is walked at the
root path, so each row repeats the binding-level name box under the one already
at the top. If per-row controls are intended they need to follow each other's
edits; otherwise decorating the template once (or only the first row) avoids
both.
--
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]
