feat: Create dataset header component #21189

lyndsiWilliams · 2022-08-24T22:02:51Z

SUMMARY

This is the blank state implementation of the header component for the create dataset page. This PR also adds an option tooltip to the button in the PageHeaderWithActions component.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

BEFORE:

AFTER:

TESTING INSTRUCTIONS

Go to http://localhost:9000/dataset/add/?testing
Observe the header component
Click the dataset name textbox and change the text by either pressing enter or clicking out of the box
- Observe the text change
Mousehover over the disabled save and menu buttons on the left
- Observe the tooltip

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

codecov · 2022-08-24T22:04:03Z

Codecov Report

Merging #21189 (c3e3544) into master (973d870) will decrease coverage by 0.01%.
The diff coverage is 95.65%.

@@            Coverage Diff             @@
##           master   #21189      +/-   ##
==========================================
- Coverage   66.61%   66.59%   -0.02%     
==========================================
  Files        1791     1791              
  Lines       68542    68554      +12     
  Branches     7318     7319       +1     
==========================================
- Hits        45656    45653       -3     
- Misses      20987    21008      +21     
+ Partials     1899     1893       -6

Flag	Coverage Δ
hive	`53.06% <ø> (ø)`
javascript	`52.76% <95.65%> (-0.03%)`	⬇️
mysql	`78.17% <ø> (ø)`
postgres	`78.23% <ø> (ø)`
presto	`52.96% <ø> (ø)`
python	`81.38% <ø> (ø)`
sqlite	`76.73% <ø> (ø)`
unit	`51.07% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...end/src/components/PageHeaderWithActions/index.tsx	`90.00% <ø> (ø)`
superset-frontend/src/components/Tooltip/index.tsx	`100.00% <ø> (ø)`
...iews/CRUD/data/dataset/AddDataset/Header/index.tsx	`87.50% <87.50%> (-12.50%)`	⬇️
...d/src/views/CRUD/data/dataset/AddDataset/index.tsx	`42.85% <100.00%> (+9.52%)`	⬆️
...rc/views/CRUD/data/dataset/DatasetLayout/index.tsx	`100.00% <100.00%> (ø)`
...set-frontend/src/views/CRUD/data/dataset/styles.ts	`100.00% <100.00%> (ø)`
...rd/components/nativeFilters/FilterBar/keyValue.tsx	`18.51% <0.00%> (-22.23%)`	⬇️
...board/components/nativeFilters/FilterBar/index.tsx	`60.14% <0.00%> (-5.80%)`	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

github-actions · 2022-08-24T22:09:37Z

Storybook has completed and can be viewed at

lyndsiWilliams · 2022-08-24T23:04:59Z

/testenv up

github-actions · 2022-08-24T23:07:01Z

@lyndsiWilliams Ephemeral environment spinning up at http://54.213.235.192:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

geido · 2022-08-25T12:15:28Z

superset-frontend/src/components/PageHeaderWithActions/index.tsx

+ tooltip={tooltipProps?.text ?? undefined}
+ placement={tooltipProps?.placement ?? undefined}


Suggested change

tooltip={tooltipProps?.text ?? undefined}

placement={tooltipProps?.placement ?? undefined}

tooltip={tooltipProps?.text}

placement={tooltipProps?.placement}

Fixed in this commit.

geido · 2022-08-25T12:19:53Z

superset-frontend/src/components/PageHeaderWithActions/index.tsx

@@ -19,6 +19,7 @@
 import React, { ReactNode, ReactElement } from 'react';
 import { css, SupersetTheme, t, useTheme } from '@superset-ui/core';
 import { AntdDropdown, AntdDropdownProps } from 'src/components';
+import { TooltipPlacement } from 'antd/es/tooltip';


Would you mind exporting this from the Tooltip component in our components library and then importing it here? I think we should centralize all the components related things and avoid accessing antd directly

Good thinking! Fixed in this commit.

geido · 2022-08-25T12:23:58Z

superset-frontend/src/views/CRUD/data/dataset/AddDataset/AddDataset.test.tsx

 import AddDataset from 'src/views/CRUD/data/dataset/AddDataset';

 describe('AddDataset', () => {
- it('renders a blank state AddDataset', () => {
- render(<AddDataset />);
+ const renderAndWait = async () => {


If the purpose here to wait until the component has finished loading, you can use something like this

const waitForRender = () => waitFor(() => render(<AddDataset />)); await waitForRender();

Oh yeah that's a lot cleaner! Fixed in this commit.

geido · 2022-08-25T12:24:50Z

superset-frontend/src/views/CRUD/data/dataset/AddDataset/Header/index.tsx

+
+const editableTitleProps = {
+ title: '',
+ placeholder: 'Add the name of the dataset',


I think all the text in this file needs to be localized

Good call, fixed in this commit.

AAfghahi · 2022-08-25T14:34:32Z

superset-frontend/src/views/CRUD/data/dataset/AddDataset/Header/index.tsx

+ placement: 'bottomRight',
+};
+
+const Styles = styled.div`


should this go into styles?

Oh yeah good call, fixed in this commit.

AAfghahi · 2022-08-25T14:34:47Z

superset-frontend/src/views/CRUD/data/dataset/AddDataset/Header/index.tsx

+ }
+`;
+
+const disabledSaveBtnStyles = (theme: SupersetTheme) => css`


same with this.

Also fixed in this commit.

AAfghahi · 2022-08-25T14:39:40Z

superset-frontend/src/views/CRUD/data/dataset/AddDataset/Header/index.tsx


 export default function Header() {
- return <div>Header</div>;
+ return (


hey, quick question in regards to the save button. What happens, currently, when I press save? I believe that we have a state action that is supposed to put the name into state. Does it currently do that?

Good call, I have the data properly flowing now in this commit.

geido · 2022-08-30T11:12:16Z

/testenv up

github-actions · 2022-08-30T11:40:01Z

@geido Ephemeral environment spinning up at http://35.91.130.23:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

AAfghahi · 2022-08-30T14:41:58Z

superset-frontend/src/views/CRUD/data/dataset/AddDataset/Header/Header.test.tsx

+ expect(saveButton).toBeVisible();
+ expect(saveButton).toBeDisabled();
+ expect(menuButton).toBeVisible();
+ expect(menuButton).toBeDisabled();


should there be a test for when someone passes in a name?

Can do! Added in this commit.

hughhhh · 2022-09-08T19:33:54Z

superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/DatasetLayout.test.tsx

- expect(screen.getByText(/header/i)).toBeVisible();
+ const waitForRender = () =>
+ waitFor(() =>
+ render(<Header setDataset={mockSetDataset} datasetName="" />),


add a test to make sure if datasetName is set that value of the textbox renders

That is covered in these tests: https://github.com/apache/superset/pull/21189/files#diff-2f904a31ce40cd3a52f574acd3093d7bb41d513ec824eb6e61693b52c0203920R52-R78

jinghua-qa · 2022-09-08T22:52:43Z

/testenv up

github-actions · 2022-09-08T22:56:50Z

@jinghua-qa Ephemeral environment spinning up at http://35.91.138.110:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

jinghua-qa · 2022-09-09T04:44:50Z

LGTM

github-actions · 2022-09-14T00:39:21Z

Ephemeral environment shutdown and build artifacts deleted.

C-monC · 2022-09-15T16:57:49Z

Any possibility this induces #21476?

pull-request-size bot added the size/L label Aug 24, 2022

geido reviewed Aug 25, 2022

View reviewed changes

AAfghahi reviewed Aug 25, 2022

View reviewed changes

AAfghahi approved these changes Aug 30, 2022

View reviewed changes

lyndsiWilliams added the need:qa-review Requires QA review label Aug 31, 2022

hughhhh reviewed Sep 8, 2022

View reviewed changes

AAfghahi mentioned this pull request Sep 8, 2022

feat: Dataset Creation Footer Component #21241

Merged

9 tasks

hughhhh approved these changes Sep 13, 2022

View reviewed changes

lyndsiWilliams added 5 commits September 13, 2022 16:37

Created blank header component

ee1eabd

Make tooltipProps optional

4c1cdb7

Add disabled btn tests

c391dc9

address review comments

0557fba

add testing for Header title

1389b8b

lyndsiWilliams force-pushed the lyndsi/create-dataset-header branch from c3a9485 to 1389b8b Compare September 13, 2022 21:40

fix addDataset test

c3e3544

lyndsiWilliams merged commit 6e8cad3 into master Sep 14, 2022

lyndsiWilliams deleted the lyndsi/create-dataset-header branch September 14, 2022 01:47

mistercrunch added the 🚢 2.1.3 label Feb 18, 2024

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 and removed 🚢 2.1.3 labels Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Create dataset header component #21189

feat: Create dataset header component #21189

lyndsiWilliams commented Aug 24, 2022

codecov bot commented Aug 24, 2022 •

edited

Loading

github-actions bot commented Aug 24, 2022

lyndsiWilliams commented Aug 24, 2022

github-actions bot commented Aug 24, 2022

geido Aug 25, 2022

lyndsiWilliams Aug 29, 2022

geido Aug 25, 2022

lyndsiWilliams Aug 29, 2022

geido Aug 25, 2022 •

edited

Loading

lyndsiWilliams Aug 29, 2022

geido Aug 25, 2022

lyndsiWilliams Aug 29, 2022

AAfghahi Aug 25, 2022

lyndsiWilliams Aug 29, 2022

AAfghahi Aug 25, 2022

lyndsiWilliams Aug 29, 2022

AAfghahi Aug 25, 2022

lyndsiWilliams Aug 29, 2022

geido commented Aug 30, 2022

github-actions bot commented Aug 30, 2022

AAfghahi Aug 30, 2022

lyndsiWilliams Aug 31, 2022

hughhhh Sep 8, 2022 •

edited

Loading

lyndsiWilliams Sep 13, 2022

jinghua-qa commented Sep 8, 2022

github-actions bot commented Sep 8, 2022

jinghua-qa commented Sep 9, 2022

github-actions bot commented Sep 14, 2022

C-monC commented Sep 15, 2022

		tooltip={tooltipProps?.text ?? undefined}
		placement={tooltipProps?.placement ?? undefined}

feat: Create dataset header component #21189

feat: Create dataset header component #21189

Conversation

lyndsiWilliams commented Aug 24, 2022

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

BEFORE:

AFTER:

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Aug 24, 2022 • edited Loading

Codecov Report

github-actions bot commented Aug 24, 2022

lyndsiWilliams commented Aug 24, 2022

github-actions bot commented Aug 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geido Aug 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geido commented Aug 30, 2022

github-actions bot commented Aug 30, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hughhhh Sep 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jinghua-qa commented Sep 8, 2022

github-actions bot commented Sep 8, 2022

jinghua-qa commented Sep 9, 2022

github-actions bot commented Sep 14, 2022

C-monC commented Sep 15, 2022

codecov bot commented Aug 24, 2022 •

edited

Loading

geido Aug 25, 2022 •

edited

Loading

hughhhh Sep 8, 2022 •

edited

Loading