SBIN2010 commented on PR #35897:
URL: https://github.com/apache/superset/pull/35897#issuecomment-3880311145
> ## 1. Form State Sync Issue
> The checkbox handlers (setToAllRow, setToTextColor, etc.) update local
React state but don't call form.setFieldsValue() to sync the Form's internal
state. This can cause the form to submit stale values. For example:
>
> ```
> // Current pattern - only updates local state
> onChange={event => setToAllRow(event.target.checked)}
>
> // Should also sync form state
> onChange={event => {
> const checked = event.target.checked;
> setToAllRow(checked);
> form.setFieldsValue({ toAllRow: checked });
> }}
> ```
Interesting note.
setToAllRow and setToTextColor have been replaced with
setColumnFormatin and setObjectFormating.
Accordingly, the checkboxes for toAllRow and toTextColor have been removed
from the conditional formatting interface.
toAllRow and toTextColor have been retained in the table's
ConditionalFormattingConfig for a smooth transition to the new objects.
>. Mixed Naming Patterns
>The PR introduces objectFormatting (enum-based) alongside the existing
toAllRow/toTextColor
>(boolean) patterns. This creates two ways to express the same concept:
>
> // New pattern
> formatter.objectFormatting === ObjectFormattingEnum.TEXT_COLOR
>
> // Old pattern still used
> formatter.toTextColor
>Could we consolidate on one approach? Either:
>
>Migrate fully to objectFormatting and deprecate the boolean flags, or
>Keep the boolean flags and skip the new enum
>This will make the code easier to maintain and avoid confusion about which
pattern to check.
>
The purpose of the replacement is a complete transition to formatting
objects and removing logical flags, but more smoothly, with the transfer of
parameters if such flags have already been used.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
