Re: [PR] fix(theming): Superset Theme submenu UI alignment [superset]

[via GitHub]

korbit-ai[bot] commented on code in PR #34430:
URL: https://github.com/apache/superset/pull/34430#discussion_r2245684037


##########
superset-frontend/src/features/home/RightMenu.tsx:
##########
@@ -373,6 +397,74 @@ const RightMenu = ({
     localStorage.removeItem('redux');
   };
 
+  const handleThemeSelect = useCallback(
+    (mode: ThemeMode) => {
+      setThemeMode(mode);
+    },
+    [setThemeMode],
+  );
+
+  const themeIconMap: Record<ThemeAlgorithm | ThemeMode, React.ReactNode> =
+    useMemo(
+      () => ({
+        [ThemeAlgorithm.DEFAULT]: <Icons.SunOutlined />,
+        [ThemeAlgorithm.DARK]: <Icons.MoonOutlined />,
+        [ThemeMode.SYSTEM]: <Icons.FormatPainterOutlined />,
+        [ThemeAlgorithm.COMPACT]: <Icons.CompressOutlined />,
+      }),
+      [],
+    );

Review Comment:
   ### Inconsistent Theme Enum Usage <sub>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The themeIconMap is mixing ThemeAlgorithm and ThemeMode enums incorrectly. 
ThemeMode.DEFAULT should be used instead of ThemeAlgorithm.DEFAULT for 
consistency with the theme options.
   
   
   ###### Why this matters
   This mismatch between ThemeAlgorithm and ThemeMode could cause the wrong 
icon to be displayed for the light theme mode, as the selected icon is derived 
from the themeMode value which uses ThemeMode enum.
   
   ###### Suggested change ∙ *Feature Preview*
   ```typescript
   const themeIconMap: Record<ThemeMode, React.ReactNode> = useMemo(
     () => ({
       [ThemeMode.DEFAULT]: <Icons.SunOutlined />,
       [ThemeMode.DARK]: <Icons.MoonOutlined />,
       [ThemeMode.SYSTEM]: <Icons.FormatPainterOutlined />,
     }),
     [],
   );
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/68b88ac7-2725-4f3b-8ad5-d362b1c878f7/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/68b88ac7-2725-4f3b-8ad5-d362b1c878f7?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/68b88ac7-2725-4f3b-8ad5-d362b1c878f7?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/68b88ac7-2725-4f3b-8ad5-d362b1c878f7?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/68b88ac7-2725-4f3b-8ad5-d362b1c878f7)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:d4fddaec-f36c-4ad6-9d6a-904057b47fc1 -->
   
   
   [](d4fddaec-f36c-4ad6-9d6a-904057b47fc1)



-- 
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-unsubscr...@superset.apache.org

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


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