[GitHub] [superset] justinpark commented on pull request #21819: feat(dashboard): confirm overwrite to prevent unintended changes

[GitBox <gi...@apache.org>]

justinpark commented on PR #21819:
URL: https://github.com/apache/superset/pull/21819#issuecomment-1301098877

   > > 1 - Can we rename the feature flag to a shorter name like `DASHBOARD_METADATA_DIFF` or anything else?
   > 
   > I updated to `CONFIRM_DASHBOARD_DIFF`
   > 
   > > 2 - Can you add the new feature flag to `RESOURCES/FEATURE_FLAGS.md`?
   > 
   > Yes. updated.
   > 
   > > 3 - Can we set a min/max width/height to the modal to avoid the situation where it takes the whole screen?
   > 
   > Yes I added the maxWidth using `responsive`. (its size up to 1024px)
   > 
   > > 4 - Can we make the modal appear only when there's a change to the scope or CSS? Unrelated changes shouldn't present it.
   > 
   > Sure. The modal only pops up when [the following values](https://github.com/apache/superset/pull/21819/files#diff-c070f1a491b40805e52139c971994f682921d2588cf1f06685d30ccf200cd419R44) are changed.
   > 
   > `export const OVERWRITE_INSPECT_FIELDS = ['css', 'json_metadata.filter_scopes'];`
   > 
   > > 5 - Is it possible to make the scope changes more user-friendly? It's showing ID diffs that are not traceable to the screen. Maybe add the chart names to the diff or use a different kind of scope representation when comparing.
   > 
   > as ktmud commented, it becomes complicated to access the scope labels and its hierarchy. I'll follow up this improvement in the next step.
   
   @michael-s-molina do you have any objections with the approach? We'd like to kick off the feature with JSON diffs first. (and then will follow up the improved labels version)


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org