[GitHub] [superset] geido commented on a diff in pull request #20612: chore(explore): update Explore icons and icon colors

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

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


##########
superset-frontend/src/explore/components/ControlHeader.tsx:
##########
@@ -55,6 +59,18 @@ const ControlHeader: FC<ControlHeaderProps> = ({
   danger,
 }) => {
   const { gridUnit, colors } = useTheme();
+  const hasHadNoErrors = useRef(false);
+  const labelColor = useMemo(() => {
+    if (!validationErrors.length) {
+      hasHadNoErrors.current = true;
+    }
+
+    return hasHadNoErrors.current

Review Comment:
   We tend to avoid doing this. It would be better to unpack this logic for better readability



##########
superset-frontend/src/explore/components/ControlPanelsContainer.tsx:
##########
@@ -58,6 +58,11 @@ import { ChartState, ExplorePageState } from 'src/explore/types';
 import { Tooltip } from 'src/components/Tooltip';
 
 import { rgba } from 'emotion-rgba';
+import { kebabCase } from 'lodash';
+import {
+  ExclamationCircleOutlined,
+  InfoCircleOutlined,
+} from '@ant-design/icons';

Review Comment:
   We don't import icons directly from Antdesign. You need to use the _Icons_ component that comes with Antdesign icons built-in as well



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