Copilot commented on code in PR #37304:
URL: https://github.com/apache/superset/pull/37304#discussion_r2713944139
##########
superset-frontend/src/components/ListView/ListView.tsx:
##########
@@ -197,32 +200,44 @@ const ViewModeToggle = ({
}: {
mode: 'table' | 'card';
setMode: (mode: 'table' | 'card') => void;
-}) => (
- <ViewModeContainer>
- <div
- role="button"
- tabIndex={0}
- onClick={e => {
- e.currentTarget.blur();
- setMode('card');
- }}
- className={cx('toggle-button', { active: mode === 'card' })}
- >
- <Icons.AppstoreOutlined iconSize="xl" />
- </div>
- <div
- role="button"
- tabIndex={0}
- onClick={e => {
- e.currentTarget.blur();
- setMode('table');
- }}
- className={cx('toggle-button', { active: mode === 'table' })}
- >
- <Icons.UnorderedListOutlined iconSize="xl" />
- </div>
- </ViewModeContainer>
-);
+}) => {
+
+
+ return (
+ <ViewModeContainer>
+ <Tooltip title={gridTitle}>
Review Comment:
The variable 'gridTitle' is referenced but not defined. You need to define
this variable with the appropriate tooltip text. Based on the context, this
should use the t() function for internationalization, for example: const
gridTitle = t('Grid view');
##########
superset-frontend/src/components/ListView/ListView.tsx:
##########
@@ -197,32 +200,44 @@ const ViewModeToggle = ({
}: {
mode: 'table' | 'card';
setMode: (mode: 'table' | 'card') => void;
-}) => (
- <ViewModeContainer>
- <div
- role="button"
- tabIndex={0}
- onClick={e => {
- e.currentTarget.blur();
- setMode('card');
- }}
- className={cx('toggle-button', { active: mode === 'card' })}
- >
- <Icons.AppstoreOutlined iconSize="xl" />
- </div>
- <div
- role="button"
- tabIndex={0}
- onClick={e => {
- e.currentTarget.blur();
- setMode('table');
- }}
- className={cx('toggle-button', { active: mode === 'table' })}
- >
- <Icons.UnorderedListOutlined iconSize="xl" />
- </div>
- </ViewModeContainer>
-);
+}) => {
+
+
+ return (
+ <ViewModeContainer>
+ <Tooltip title={gridTitle}>
+ <div
+ role="button"
+ tabIndex={0}
+ onClick={e => {
+ console.log('Grid view icon clicked');
+ e.currentTarget.blur();
+ setMode('card');
+ }}
+ onMouseEnter={() => console.log('Mouse entered Grid view icon')}
+ className={cx('toggle-button', { active: mode === 'card' })}
+ >
+ <Icons.AppstoreOutlined iconSize="xl" />
+ </div>
+ </Tooltip>
+ <Tooltip title={listTitle}>
+ <div
+ role="button"
+ tabIndex={0}
+ onClick={e => {
+ console.log('List view icon clicked');
Review Comment:
Remove this console.log debugging statement before merging. Debugging code
should not be present in production code.
##########
superset-frontend/src/components/ListView/ListView.tsx:
##########
@@ -197,32 +200,44 @@ const ViewModeToggle = ({
}: {
mode: 'table' | 'card';
setMode: (mode: 'table' | 'card') => void;
-}) => (
- <ViewModeContainer>
- <div
- role="button"
- tabIndex={0}
- onClick={e => {
- e.currentTarget.blur();
- setMode('card');
- }}
- className={cx('toggle-button', { active: mode === 'card' })}
- >
- <Icons.AppstoreOutlined iconSize="xl" />
- </div>
- <div
- role="button"
- tabIndex={0}
- onClick={e => {
- e.currentTarget.blur();
- setMode('table');
- }}
- className={cx('toggle-button', { active: mode === 'table' })}
- >
- <Icons.UnorderedListOutlined iconSize="xl" />
- </div>
- </ViewModeContainer>
-);
+}) => {
+
+
+ return (
+ <ViewModeContainer>
+ <Tooltip title={gridTitle}>
+ <div
+ role="button"
+ tabIndex={0}
+ onClick={e => {
+ console.log('Grid view icon clicked');
+ e.currentTarget.blur();
+ setMode('card');
+ }}
+ onMouseEnter={() => console.log('Mouse entered Grid view icon')}
+ className={cx('toggle-button', { active: mode === 'card' })}
+ >
+ <Icons.AppstoreOutlined iconSize="xl" />
+ </div>
+ </Tooltip>
+ <Tooltip title={listTitle}>
+ <div
+ role="button"
+ tabIndex={0}
+ onClick={e => {
+ console.log('List view icon clicked');
+ e.currentTarget.blur();
+ setMode('table');
+ }}
+ onMouseEnter={() => console.log('Mouse entered List view icon')}
Review Comment:
Remove this console.log debugging statement before merging. Debugging code
should not be present in production code.
##########
superset-frontend/src/components/ListView/ListView.tsx:
##########
@@ -197,32 +200,44 @@ const ViewModeToggle = ({
}: {
mode: 'table' | 'card';
setMode: (mode: 'table' | 'card') => void;
-}) => (
- <ViewModeContainer>
- <div
- role="button"
- tabIndex={0}
- onClick={e => {
- e.currentTarget.blur();
- setMode('card');
- }}
- className={cx('toggle-button', { active: mode === 'card' })}
- >
- <Icons.AppstoreOutlined iconSize="xl" />
- </div>
- <div
- role="button"
- tabIndex={0}
- onClick={e => {
- e.currentTarget.blur();
- setMode('table');
- }}
- className={cx('toggle-button', { active: mode === 'table' })}
- >
- <Icons.UnorderedListOutlined iconSize="xl" />
- </div>
- </ViewModeContainer>
-);
+}) => {
+
+
+ return (
+ <ViewModeContainer>
+ <Tooltip title={gridTitle}>
+ <div
+ role="button"
+ tabIndex={0}
+ onClick={e => {
+ console.log('Grid view icon clicked');
+ e.currentTarget.blur();
+ setMode('card');
+ }}
+ onMouseEnter={() => console.log('Mouse entered Grid view icon')}
+ className={cx('toggle-button', { active: mode === 'card' })}
+ >
+ <Icons.AppstoreOutlined iconSize="xl" />
+ </div>
+ </Tooltip>
+ <Tooltip title={listTitle}>
Review Comment:
The variable 'listTitle' is referenced but not defined. You need to define
this variable with the appropriate tooltip text. Based on the context, this
should use the t() function for internationalization, for example: const
listTitle = t('List view');
##########
superset-frontend/src/components/ListView/ListView.tsx:
##########
@@ -270,7 +285,7 @@ export function ListView<T extends object = any>({
filters = [],
bulkActions = [],
bulkSelectEnabled = false,
- disableBulkSelect = () => {},
+ disableBulkSelect = () => { },
Review Comment:
This spacing change appears unintentional. The arrow function formatting
should remain consistent with the rest of the codebase. Revert this change to
the original formatting: () => {}
```suggestion
disableBulkSelect = () => {},
```
##########
superset-frontend/src/components/ListView/ListView.tsx:
##########
@@ -197,32 +200,44 @@ const ViewModeToggle = ({
}: {
mode: 'table' | 'card';
setMode: (mode: 'table' | 'card') => void;
-}) => (
- <ViewModeContainer>
- <div
- role="button"
- tabIndex={0}
- onClick={e => {
- e.currentTarget.blur();
- setMode('card');
- }}
- className={cx('toggle-button', { active: mode === 'card' })}
- >
- <Icons.AppstoreOutlined iconSize="xl" />
- </div>
- <div
- role="button"
- tabIndex={0}
- onClick={e => {
- e.currentTarget.blur();
- setMode('table');
- }}
- className={cx('toggle-button', { active: mode === 'table' })}
- >
- <Icons.UnorderedListOutlined iconSize="xl" />
- </div>
- </ViewModeContainer>
-);
+}) => {
+
+
+ return (
+ <ViewModeContainer>
+ <Tooltip title={gridTitle}>
+ <div
+ role="button"
+ tabIndex={0}
+ onClick={e => {
+ console.log('Grid view icon clicked');
+ e.currentTarget.blur();
+ setMode('card');
+ }}
+ onMouseEnter={() => console.log('Mouse entered Grid view icon')}
Review Comment:
Remove this console.log debugging statement before merging. Debugging code
should not be present in production code.
##########
superset-frontend/src/components/ListView/ListView.tsx:
##########
@@ -197,32 +200,44 @@ const ViewModeToggle = ({
}: {
mode: 'table' | 'card';
setMode: (mode: 'table' | 'card') => void;
-}) => (
- <ViewModeContainer>
- <div
- role="button"
- tabIndex={0}
- onClick={e => {
- e.currentTarget.blur();
- setMode('card');
- }}
- className={cx('toggle-button', { active: mode === 'card' })}
- >
- <Icons.AppstoreOutlined iconSize="xl" />
- </div>
- <div
- role="button"
- tabIndex={0}
- onClick={e => {
- e.currentTarget.blur();
- setMode('table');
- }}
- className={cx('toggle-button', { active: mode === 'table' })}
- >
- <Icons.UnorderedListOutlined iconSize="xl" />
- </div>
- </ViewModeContainer>
-);
+}) => {
+
+
+ return (
+ <ViewModeContainer>
+ <Tooltip title={gridTitle}>
+ <div
+ role="button"
+ tabIndex={0}
+ onClick={e => {
+ console.log('Grid view icon clicked');
Review Comment:
Remove this console.log debugging statement before merging. Debugging code
should not be present in production code.
##########
superset-frontend/src/components/ListView/ListView.tsx:
##########
@@ -28,8 +28,11 @@ import {
Icons,
EmptyState,
Loading,
+ Tooltip,
type EmptyStateProps,
} from '@superset-ui/core/components';
+
+
Review Comment:
Remove these unnecessary blank lines to maintain consistency with the
codebase style.
```suggestion
```
##########
superset-frontend/src/components/ListView/ListView.tsx:
##########
@@ -197,32 +200,44 @@ const ViewModeToggle = ({
}: {
mode: 'table' | 'card';
setMode: (mode: 'table' | 'card') => void;
-}) => (
- <ViewModeContainer>
- <div
- role="button"
- tabIndex={0}
- onClick={e => {
- e.currentTarget.blur();
- setMode('card');
- }}
- className={cx('toggle-button', { active: mode === 'card' })}
- >
- <Icons.AppstoreOutlined iconSize="xl" />
- </div>
- <div
- role="button"
- tabIndex={0}
- onClick={e => {
- e.currentTarget.blur();
- setMode('table');
- }}
- className={cx('toggle-button', { active: mode === 'table' })}
- >
- <Icons.UnorderedListOutlined iconSize="xl" />
- </div>
- </ViewModeContainer>
-);
+}) => {
+
+
Review Comment:
Remove these unnecessary blank lines to maintain consistency with the
codebase style.
--
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]
