msyavuz commented on code in PR #36416:
URL: https://github.com/apache/superset/pull/36416#discussion_r2681489144


##########
superset-frontend/plugins/plugin-chart-table/src/transformProps.ts:
##########
@@ -276,10 +287,25 @@ const processColumns = memoizeOne(function processColumns(
         // percent metrics have a default format
         formatter = getNumberFormatter(numberFormat || PERCENT_3_POINT);
       } else if (isMetric || (isNumber && (numberFormat || currency))) {
-        formatter = currency?.symbol
+        // Resolve AUTO currency when currency column isn't in query results
+        let resolvedCurrency = currency;

Review Comment:
   Is this backwards compatible? Meaning if someone who doesn't have a 
currency_column in their data keeps their previously set currency?



##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.jsx:
##########
@@ -403,15 +430,13 @@ function ColumnCollectionTable({
               type: t('Data type'),
               groupby: t('Is dimension'),
               is_dttm: t('Is temporal'),
-              main_dttm_col: t('Default datetime'),

Review Comment:
   Is losing this significant?



##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.jsx:
##########
@@ -157,6 +159,47 @@ const StyledTableTabWrapper = styled.div`
   }
 `;
 
+const DefaultColumnSettingsContainer = styled.div`
+  ${({ theme }) => css`
+    margin-bottom: ${theme.sizeUnit * 4}px;
+  `}
+`;
+
+const DefaultColumnSettingsTitle = styled.h4`
+  ${({ theme }) => css`
+    margin: 0 0 ${theme.sizeUnit * 2}px 0;
+    font-size: ${theme.fontSizeLG}px;
+    font-weight: ${theme.fontWeightMedium};
+    color: ${theme.colorText};
+  `}
+`;
+
+const DefaultColumnSettingsFields = styled.div`
+  ${({ theme }) => css`
+    display: flex;
+    flex-direction: column;
+    gap: ${theme.sizeUnit * 3}px;
+  `}
+`;
+
+const FieldWrapper = styled.div`
+  ${({ theme }) => css`
+    display: flex;
+    flex-direction: column;
+    gap: ${theme.sizeUnit}px;
+  `}
+`;
+
+const FieldLabelWithTooltip = styled.div`
+  ${({ theme }) => css`
+    display: flex;
+    align-items: center;
+    gap: ${theme.sizeUnit}px;
+    font-size: ${theme.fontSizeSM}px;
+    color: ${theme.colorTextLabel};
+  `}
+`;

Review Comment:
   Seems like a good place to use our Flex component?



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