msyavuz commented on code in PR #35129:
URL: https://github.com/apache/superset/pull/35129#discussion_r2378730167
##########
superset-frontend/packages/superset-ui-core/src/components/Modal/Modal.tsx:
##########
@@ -368,9 +368,13 @@ const CustomModal = ({
};
CustomModal.displayName = 'Modal';
+// Theme-aware confirmation modal - now inherits theme through App wrapper in
SupersetThemeProvider
+const themedConfirm = (props: Parameters<typeof AntdModal.confirm>[0]) =>
+ AntdModal.confirm(props);
+
export const Modal = Object.assign(CustomModal, {
error: AntdModal.error,
warning: AntdModal.warning,
Review Comment:
Should these two use the themed version as well?
##########
superset/templates/superset/spa.html:
##########
@@ -85,14 +99,9 @@
<!-- Custom URL from theme -->
<img
src="{{ tokens.brandSpinnerUrl }}"
- alt="Loading..."
Review Comment:
We should probably keep this for accessibility
##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx:
##########
@@ -317,13 +317,20 @@ export default function PluginFilterSelect(props:
PluginFilterSelectProps) {
const sortComparator = useCallback(
(a: LabeledValue, b: LabeledValue) => {
+ // When sortMetric is specified, the backend already sorted the data
correctly
+ // Don't override the backend's metric-based sorting with frontend
alphabetical sorting
+ if (formData.sortMetric) {
+ return 0; // Preserve the original order from the backend
+ }
+
+ // Only apply alphabetical sorting when no sortMetric is specified
const labelComparator = propertyComparator('label');
if (formData.sortAscending) {
return labelComparator(a, b);
}
return labelComparator(b, a);
},
- [formData.sortAscending],
+ [formData.sortAscending, formData.sortMetric],
Review Comment:
This looks like leftover code from another pr
##########
superset/embedded/view.py:
##########
@@ -86,10 +85,7 @@ def embedded(
},
}
- return self.render_template(
- "superset/spa.html",
+ return self.render_app_template(
+ extra_bootstrap_data=extra_bootstrap_data,
entry="embedded",
- bootstrap_data=json.dumps(
- bootstrap_data, default=json.pessimistic_json_iso_dttm_ser
- ),
Review Comment:
Getting rid of this will make it so there will be errors when the
serialization fails
##########
superset-frontend/packages/superset-ui-core/src/theme/Theme.tsx:
##########
@@ -243,7 +243,7 @@ export class Theme {
<ThemeProvider theme={themeState.theme}>
<GlobalStyles />
<ConfigProvider theme={themeState.antdConfig}>
- {children}
+ <App>{children}</App>
Review Comment:
This provides some reset styles that could mess up other parts of the ui.
--
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]
