geido commented on code in PR #20589:
URL: https://github.com/apache/superset/pull/20589#discussion_r916101868


##########
superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx:
##########
@@ -122,10 +132,86 @@ const DashboardContainer: FC<DashboardContainerProps> = 
({ topLevelTabs }) => {
     dispatch(setInScopeStatusOfFilters(scopes));
   }, [nativeFilterScopes, dashboardLayout, dispatch]);
 
+  const verifyUpdateColorScheme = useCallback(() => {
+    const currentMetadata = dashboardInfo.metadata;
+    if (currentMetadata?.color_scheme) {
+      const metadata = { ...currentMetadata };
+      const colorScheme = metadata?.color_scheme;
+      const colorSchemeDomain = metadata?.color_scheme_domain || [];
+      const categoricalSchemes = getCategoricalSchemeRegistry();
+      const registryColorSchemeDomain =
+        categoricalSchemes.get(colorScheme)?.colors || [];
+      const defaultColorScheme = categoricalSchemes.defaultKey;
+      const isColorSchemeExisting = !!categoricalSchemes.get(colorScheme);
+      const updateDashboard = () => {
+        SupersetClient.put({
+          endpoint: `/api/v1/dashboard/${dashboardInfo.id}`,
+          headers: { 'Content-Type': 'application/json' },
+          body: JSON.stringify({
+            json_metadata: jsonStringify(metadata),
+          }),
+        }).catch(e => console.log(e));

Review Comment:
   @kgabryje this isn't really an update for which the user should have any 
feedback whether positive or negative, since this is a fallback situation when 
a color scheme is deleted or changed in the `EXTRA_CATEGORICAL_COLOR_SCHEMES` 
or `EXTRA_SEQUENTIAL_COLOR_SCHEMES` or when the dashboard does not have the 
`color_scheme_domain` definition and needs it to be compatible.
   
   As for using redux, the dashboard is updated like this in the 
PropertiesModal and other places as well. The reason why this isn't using the 
available saveDashboardRequest is because that has additional logic and user 
feedback that we don't need here, so I believe for this scenario it makes sense 
to keep it as close as possible to the fallback logic



##########
superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx:
##########
@@ -122,10 +132,86 @@ const DashboardContainer: FC<DashboardContainerProps> = 
({ topLevelTabs }) => {
     dispatch(setInScopeStatusOfFilters(scopes));
   }, [nativeFilterScopes, dashboardLayout, dispatch]);
 
+  const verifyUpdateColorScheme = useCallback(() => {
+    const currentMetadata = dashboardInfo.metadata;
+    if (currentMetadata?.color_scheme) {
+      const metadata = { ...currentMetadata };
+      const colorScheme = metadata?.color_scheme;
+      const colorSchemeDomain = metadata?.color_scheme_domain || [];
+      const categoricalSchemes = getCategoricalSchemeRegistry();
+      const registryColorSchemeDomain =
+        categoricalSchemes.get(colorScheme)?.colors || [];
+      const defaultColorScheme = categoricalSchemes.defaultKey;
+      const isColorSchemeExisting = !!categoricalSchemes.get(colorScheme);
+      const updateDashboard = () => {
+        SupersetClient.put({
+          endpoint: `/api/v1/dashboard/${dashboardInfo.id}`,
+          headers: { 'Content-Type': 'application/json' },
+          body: JSON.stringify({
+            json_metadata: jsonStringify(metadata),
+          }),
+        }).catch(e => console.log(e));

Review Comment:
   @kgabryje this isn't really an update for which the user should have any 
feedback whether positive or negative, since this is a fallback situation when 
a color scheme is deleted or changed in the `EXTRA_CATEGORICAL_COLOR_SCHEMES` 
or `EXTRA_SEQUENTIAL_COLOR_SCHEMES` or when the dashboard does not have the 
`color_scheme_domain` definition and needs it to be compatible.
   
   As for using redux, the dashboard is updated like this in the 
PropertiesModal and other places as well. The reason why this isn't using the 
available `saveDashboardRequest` is because that has additional logic and user 
feedback that we don't need here, so I believe for this scenario it makes sense 
to keep it as close as possible to the fallback logic



-- 
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]

Reply via email to