michael-s-molina commented on code in PR #31979:
URL: https://github.com/apache/superset/pull/31979#discussion_r1932761890
##########
superset-frontend/src/dashboard/components/ColorSchemeControlWrapper.jsx:
##########
@@ -17,60 +17,37 @@
* under the License.
*/
/* eslint-env browser */
-import PropTypes from 'prop-types';
-import { PureComponent } from 'react';
import { getCategoricalSchemeRegistry, t } from '@superset-ui/core';
-
import ColorSchemeControl from
'src/explore/components/controls/ColorSchemeControl';
-const propTypes = {
- onChange: PropTypes.func,
- labelMargin: PropTypes.number,
- colorScheme: PropTypes.string,
- hasCustomLabelsColor: PropTypes.bool,
-};
-
-const defaultProps = {
- hasCustomLabelsColor: false,
- colorScheme: undefined,
- onChange: () => {},
+const ColorSchemeControlWrapper = ({
Review Comment:
Could we use this opportunity to move this to a TS file?
##########
superset-frontend/src/dashboard/components/ColorSchemeControlWrapper.jsx:
##########
@@ -17,60 +17,37 @@
* under the License.
*/
/* eslint-env browser */
-import PropTypes from 'prop-types';
-import { PureComponent } from 'react';
import { getCategoricalSchemeRegistry, t } from '@superset-ui/core';
-
import ColorSchemeControl from
'src/explore/components/controls/ColorSchemeControl';
-const propTypes = {
- onChange: PropTypes.func,
- labelMargin: PropTypes.number,
- colorScheme: PropTypes.string,
- hasCustomLabelsColor: PropTypes.bool,
-};
-
-const defaultProps = {
- hasCustomLabelsColor: false,
- colorScheme: undefined,
- onChange: () => {},
+const ColorSchemeControlWrapper = ({
+ colorScheme,
+ labelMargin = 0,
+ hasCustomLabelsColor = false,
+ hovered = false,
+ onChange = () => {},
+}) => {
+ // Registry initialization
+ const categoricalSchemeRegistry = getCategoricalSchemeRegistry();
+ const choices = categoricalSchemeRegistry.keys().map(s => [s, s]);
+ const schemes = categoricalSchemeRegistry.getMap();
Review Comment:
We can move this outside the component or encapsulate this part in a
`useEffect` that runs only once. Something like:
```
const [choices, setChoices] = useState([]);
const [schemes, setSchemes] = useState({});
useEffect(() => {
// Registry initialization
const categoricalSchemeRegistry = getCategoricalSchemeRegistry();
setChoices(categoricalSchemeRegistry.keys().map(s => [s, s]));
setSchemes(categoricalSchemeRegistry.getMap());
}, []); // Empty dependency array ensures this runs only once
```
##########
superset-frontend/src/dataMask/reducer.ts:
##########
@@ -41,20 +41,18 @@ import { areObjectsEqual } from '../reduxUtils';
export function getInitialDataMask(
id?: string | number,
- moreProps?: DataMask,
-): DataMask;
-export function getInitialDataMask(
- id: string | number,
moreProps: DataMask = {},
-): DataMaskWithId {
- let otherProps = {};
- if (id) {
- otherProps = {
- id,
- };
+): DataMask | DataMaskWithId {
+ if (id === undefined) {
+ return {
+ extraFormData: {},
+ filterState: {},
+ ownState: {},
+ ...moreProps,
+ } as DataMask;
Review Comment:
I think we can make it more readable with:
```
export function getInitialDataMask(
id?: string | number,
moreProps: DataMask = {},
): DataMask | DataMaskWithId {
return {
...(id !== undefined && { id }),
extraFormData: {},
filterState: {},
ownState: {},
...moreProps,
} as DataMask | DataMaskWithId;
}
```
##########
superset-frontend/src/features/tags/tags.ts:
##########
@@ -197,7 +197,7 @@ export function fetchObjectsByTagIds(
{
tagIds = [],
types,
- }: { tagIds: number[] | undefined; types: string | null },
+ }: { tagIds: (number | undefined)[] | string; types: string | null },
Review Comment:
@hainenber Do we actually receive an array that might contain `undefined`
values?
--
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]
