You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/05/12 17:03:37 UTC

[GitHub] [superset] kgabryje opened a new pull request, #20049: feat(dashboard): Rearrange items in chart header controls dropdown

kgabryje opened a new pull request, #20049:
URL: https://github.com/apache/superset/pull/20049

   <!---
   Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
   Example:
   fix(dashboard): load charts correctly
   -->
   
   ### SUMMARY
   This PR rearranges and renames menu items in chart's controls dropdown on Dashboard.
   
   - Grouped creating permalink and sending email in "Share" submenu
   - Grouped exporting to CSV and downloading image in "Download" submenu
   - Renamed "View chart in Explore" to "Edit chart"
   - Renamed "Maximize/Minimize chart" to "Enter/Leave fullscreen"
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   Before:
   <img width="424" alt="image" src="https://user-images.githubusercontent.com/15073128/168129407-ae88c9b5-2ec2-44af-aec3-3d21282ef182.png">
   
   After:
   <img width="496" alt="image" src="https://user-images.githubusercontent.com/15073128/168129224-be08a1fe-65b6-4d0a-ae72-a821198e394d.png">
   <img width="470" alt="image" src="https://user-images.githubusercontent.com/15073128/168129279-0a086d62-612a-4ea0-8660-546efbaaa81a.png">
   
   ### TESTING INSTRUCTIONS
   <!--- Required! What steps can be taken to manually verify the changes? -->
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] 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
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] kgabryje commented on pull request #20049: feat(dashboard): Rearrange items in chart header controls dropdown

Posted by GitBox <gi...@apache.org>.

kgabryje commented on PR #20049:
URL: https://github.com/apache/superset/pull/20049#issuecomment-1125701757

   /testenv up


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] kgabryje commented on a diff in pull request #20049: feat(dashboard): Rearrange items in chart header controls dropdown

Posted by GitBox <gi...@apache.org>.

kgabryje commented on code in PR #20049:
URL: https://github.com/apache/superset/pull/20049#discussion_r872505756


##########
superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx:
##########
@@ -125,37 +125,45 @@ test('Should render default props', () => {
   delete props.sliceCanEdit;
 
   render(<SliceHeaderControls {...props} />, { useRedux: true });
-  userEvent.click(screen.getByRole('menuitem', { name: 'Maximize chart' }));
-  userEvent.click(screen.getByRole('menuitem', { name: /Force refresh/ }));
-  userEvent.click(
-    screen.getByRole('menuitem', { name: 'Toggle chart description' }),
-  );
-  userEvent.click(
-    screen.getByRole('menuitem', { name: 'View chart in Explore' }),
-  );
-  userEvent.click(screen.getByRole('menuitem', { name: 'Export CSV' }));
-  userEvent.click(screen.getByRole('menuitem', { name: /Force refresh/ }));
+  expect(
+    screen.getByRole('menuitem', { name: 'Enter fullscreen' }),
+  ).toBeInTheDocument();
+  expect(
+    screen.getByRole('menuitem', { name: /Force refresh/ }),
+  ).toBeInTheDocument();
+  expect(
+    screen.getByRole('menuitem', { name: 'Show chart description' }),
+  ).toBeInTheDocument();
+  expect(
+    screen.getByRole('menuitem', { name: 'Edit chart' }),
+  ).toBeInTheDocument();
+  expect(
+    screen.getByRole('menuitem', { name: 'Download' }),
+  ).toBeInTheDocument();
+  expect(screen.getByRole('menuitem', { name: 'Share' })).toBeInTheDocument();
 
   expect(
     screen.getByRole('button', { name: 'More Options' }),
   ).toBeInTheDocument();
   expect(screen.getByTestId('NoAnimationDropdown')).toBeInTheDocument();
 });
 
-test('Should "export to CSV"', () => {
+test('Should "export to CSV"', async () => {
   const props = createProps();
   render(<SliceHeaderControls {...props} />, { useRedux: true });
 
   expect(props.exportCSV).toBeCalledTimes(0);
-  userEvent.click(screen.getByRole('menuitem', { name: 'Export CSV' }));
+  userEvent.hover(screen.getByText('Download'));
+  userEvent.click(await screen.findByText('Export to .CSV'));
   expect(props.exportCSV).toBeCalledTimes(1);
   expect(props.exportCSV).toBeCalledWith(371);
 });
 
 test('Should not show "Export to CSV" if slice is filter box', () => {
   const props = createProps('filter_box');
   render(<SliceHeaderControls {...props} />, { useRedux: true });
-  expect(screen.queryByRole('menuitem', { name: 'Export CSV' })).toBe(null);
+  userEvent.hover(screen.getByText('Download'));

Review Comment:
   Good point! In this particular example I changed the logic so that "Download" submenu is not rendered at all for filter box. Fixed in the rest of tests



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] michael-s-molina commented on a diff in pull request #20049: feat(dashboard): Rearrange items in chart header controls dropdown

Posted by GitBox <gi...@apache.org>.

michael-s-molina commented on code in PR #20049:
URL: https://github.com/apache/superset/pull/20049#discussion_r872419428


##########
superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx:
##########
@@ -125,37 +125,45 @@ test('Should render default props', () => {
   delete props.sliceCanEdit;
 
   render(<SliceHeaderControls {...props} />, { useRedux: true });
-  userEvent.click(screen.getByRole('menuitem', { name: 'Maximize chart' }));
-  userEvent.click(screen.getByRole('menuitem', { name: /Force refresh/ }));
-  userEvent.click(
-    screen.getByRole('menuitem', { name: 'Toggle chart description' }),
-  );
-  userEvent.click(
-    screen.getByRole('menuitem', { name: 'View chart in Explore' }),
-  );
-  userEvent.click(screen.getByRole('menuitem', { name: 'Export CSV' }));
-  userEvent.click(screen.getByRole('menuitem', { name: /Force refresh/ }));
+  expect(
+    screen.getByRole('menuitem', { name: 'Enter fullscreen' }),
+  ).toBeInTheDocument();
+  expect(
+    screen.getByRole('menuitem', { name: /Force refresh/ }),
+  ).toBeInTheDocument();
+  expect(
+    screen.getByRole('menuitem', { name: 'Show chart description' }),
+  ).toBeInTheDocument();
+  expect(
+    screen.getByRole('menuitem', { name: 'Edit chart' }),
+  ).toBeInTheDocument();
+  expect(
+    screen.getByRole('menuitem', { name: 'Download' }),
+  ).toBeInTheDocument();
+  expect(screen.getByRole('menuitem', { name: 'Share' })).toBeInTheDocument();
 
   expect(
     screen.getByRole('button', { name: 'More Options' }),
   ).toBeInTheDocument();
   expect(screen.getByTestId('NoAnimationDropdown')).toBeInTheDocument();
 });
 
-test('Should "export to CSV"', () => {
+test('Should "export to CSV"', async () => {
   const props = createProps();
   render(<SliceHeaderControls {...props} />, { useRedux: true });
 
   expect(props.exportCSV).toBeCalledTimes(0);
-  userEvent.click(screen.getByRole('menuitem', { name: 'Export CSV' }));
+  userEvent.hover(screen.getByText('Download'));
+  userEvent.click(await screen.findByText('Export to .CSV'));
   expect(props.exportCSV).toBeCalledTimes(1);
   expect(props.exportCSV).toBeCalledWith(371);
 });
 
 test('Should not show "Export to CSV" if slice is filter box', () => {
   const props = createProps('filter_box');
   render(<SliceHeaderControls {...props} />, { useRedux: true });
-  expect(screen.queryByRole('menuitem', { name: 'Export CSV' })).toBe(null);
+  userEvent.hover(screen.getByText('Download'));

Review Comment:
   Here you need to `await` for the test to be valid. It's the same logic as the test above when you hover over the Download text and waits to see if the options appear.



##########
superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx:
##########
@@ -34,6 +34,7 @@ import CrossFilterScopingModal from 'src/dashboard/components/CrossFilterScoping
 import Icons from 'src/components/Icons';
 import ModalTrigger from 'src/components/ModalTrigger';
 import ViewQueryModal from 'src/explore/components/controls/ViewQueryModal';
+import { FileImageOutlined, FileOutlined } from '@ant-design/icons';

Review Comment:
   Shouldn't these come from our `Icons` component?



##########
superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx:
##########
@@ -205,18 +211,19 @@ test('Should not show export full CSV if slice is filter box', () => {
   };
   const props = createProps('filter_box');
   render(<SliceHeaderControls {...props} />, { useRedux: true });
-  expect(screen.queryByRole('menuitem', { name: 'Export full CSV' })).toBe(
+  userEvent.hover(screen.getByText('Download'));
+  expect(screen.queryByRole('menuitem', { name: 'Export to full .CSV' })).toBe(

Review Comment:
   `await` is needed here as well



##########
superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx:
##########
@@ -256,7 +258,9 @@ class SliceHeaderControls extends React.PureComponent<
           : item}
       </div>
     ));
-    const resizeLabel = isFullSize ? t('Minimize chart') : t('Maximize chart');
+    const resizeLabel = isFullSize

Review Comment:
   Maybe rename it to `fullscreenLabel`?



##########
superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx:
##########
@@ -193,7 +198,8 @@ test('Should not show export full CSV if report is not table', () => {
   };
   const props = createProps();
   render(<SliceHeaderControls {...props} />, { useRedux: true });
-  expect(screen.queryByRole('menuitem', { name: 'Export full CSV' })).toBe(
+  userEvent.hover(screen.getByText('Download'));
+  expect(screen.queryByRole('menuitem', { name: 'Export to full .CSV' })).toBe(

Review Comment:
   `await` is needed here as well



##########
superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx:
##########
@@ -165,23 +173,20 @@ test('Export full CSV is under featureflag', () => {
   };
   const props = createProps('table');
   render(<SliceHeaderControls {...props} />, { useRedux: true });
-  expect(screen.queryByRole('menuitem', { name: 'Export full CSV' })).toBe(
-    null,
-  );
+  userEvent.hover(screen.getByText('Download'));
+  expect(screen.queryByText('Export full CSV')).toBe(null);

Review Comment:
   `await` is needed here as well



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] kgabryje merged pull request #20049: feat(dashboard): Rearrange items in chart header controls dropdown

Posted by GitBox <gi...@apache.org>.

kgabryje merged PR #20049:
URL: https://github.com/apache/superset/pull/20049


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] kasiazjc commented on pull request #20049: feat(dashboard): Rearrange items in chart header controls dropdown

Posted by GitBox <gi...@apache.org>.

kasiazjc commented on PR #20049:
URL: https://github.com/apache/superset/pull/20049#issuecomment-1126139642

   LGTM thank you 🙏 


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] kgabryje commented on a diff in pull request #20049: feat(dashboard): Rearrange items in chart header controls dropdown

Posted by GitBox <gi...@apache.org>.

kgabryje commented on code in PR #20049:
URL: https://github.com/apache/superset/pull/20049#discussion_r872504180


##########
superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx:
##########
@@ -34,6 +34,7 @@ import CrossFilterScopingModal from 'src/dashboard/components/CrossFilterScoping
 import Icons from 'src/components/Icons';
 import ModalTrigger from 'src/components/ModalTrigger';
 import ViewQueryModal from 'src/explore/components/controls/ViewQueryModal';
+import { FileImageOutlined, FileOutlined } from '@ant-design/icons';

Review Comment:
   Yeah, it got auto-imported when I copy pasted from another file. Fixed



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] github-actions[bot] commented on pull request #20049: feat(dashboard): Rearrange items in chart header controls dropdown

Posted by GitBox <gi...@apache.org>.

github-actions[bot] commented on PR #20049:
URL: https://github.com/apache/superset/pull/20049#issuecomment-1126260440

   @michael-s-molina Ephemeral environment spinning up at http://34.222.140.234:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] michael-s-molina commented on pull request #20049: feat(dashboard): Rearrange items in chart header controls dropdown

Posted by GitBox <gi...@apache.org>.

michael-s-molina commented on PR #20049:
URL: https://github.com/apache/superset/pull/20049#issuecomment-1126248100

   /testenv up


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] github-actions[bot] commented on pull request #20049: feat(dashboard): Rearrange items in chart header controls dropdown

Posted by GitBox <gi...@apache.org>.

github-actions[bot] commented on PR #20049:
URL: https://github.com/apache/superset/pull/20049#issuecomment-1125702870

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


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] kgabryje commented on pull request #20049: feat(dashboard): Rearrange items in chart header controls dropdown

Posted by GitBox <gi...@apache.org>.

kgabryje commented on PR #20049:
URL: https://github.com/apache/superset/pull/20049#issuecomment-1126223507

   > I'm assuming that we will have another one coming for the dashboard options as well (to make them consistent).
   
   Yep! That will happen when new Dashboard header (consistent with header in Explore) gets implemented


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] codecov[bot] commented on pull request #20049: feat(dashboard): Rearrange items in chart header controls dropdown

Posted by GitBox <gi...@apache.org>.

codecov[bot] commented on PR #20049:
URL: https://github.com/apache/superset/pull/20049#issuecomment-1125260876

   # [Codecov](https://codecov.io/gh/apache/superset/pull/20049?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#20049](https://codecov.io/gh/apache/superset/pull/20049?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2d688c8) into [master](https://codecov.io/gh/apache/superset/commit/ee178f47e1be56a90e5d7a549d5d47a5a805012f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ee178f4) will **decrease** coverage by `0.00%`.
   > The diff coverage is `83.33%`.
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #20049      +/-   ##
   ==========================================
   - Coverage   66.35%   66.35%   -0.01%     
   ==========================================
     Files        1712     1712              
     Lines       64083    64085       +2     
     Branches     6744     6746       +2     
   ==========================================
   - Hits        42523    42522       -1     
   - Misses      19847    19849       +2     
   - Partials     1713     1714       +1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `51.31% <83.33%> (-0.01%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/20049?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...end/src/dashboard/components/SliceHeader/index.tsx](https://codecov.io/gh/apache/superset/pull/20049/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1NsaWNlSGVhZGVyL2luZGV4LnRzeA==) | `87.23% <ø> (ø)` | |
   | [.../src/dashboard/components/gridComponents/Chart.jsx](https://codecov.io/gh/apache/superset/pull/20049/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0NoYXJ0LmpzeA==) | `60.60% <ø> (ø)` | |
   | [...dashboard/components/SliceHeaderControls/index.tsx](https://codecov.io/gh/apache/superset/pull/20049/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1NsaWNlSGVhZGVyQ29udHJvbHMvaW5kZXgudHN4) | `63.41% <83.33%> (-2.84%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/20049?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/20049?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [ee178f4...2d688c8](https://codecov.io/gh/apache/superset/pull/20049?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] kgabryje commented on pull request #20049: feat(dashboard): Rearrange items in chart header controls dropdown

Posted by GitBox <gi...@apache.org>.

kgabryje commented on PR #20049:
URL: https://github.com/apache/superset/pull/20049#issuecomment-1126165224

   > Thanks for the PR @kgabryje. I'm assuming that we will have another one coming for the dashboard options as well (to make them consistent).
   > 
   > I'm not sure if it's related but clicking on Download as image is raising the following exception:
   > 
   > <img alt="Screen Shot 2022-05-13 at 10 43 36 AM" width="526" src="https://user-images.githubusercontent.com/70410625/168296919-b3fffbdf-5c93-4c60-a1c4-e7d5e0866e92.png">
   
   Well spotted. The problem was that the menu renders in the chart holder in DOM tree, but submenu is attached to the body and there's no way to specify popup container. I changed the screenshot selector from relative to absolute (each chart holder has a class specific to chart id, so it's easy to use exact selector to grab correct chart)


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] github-actions[bot] commented on pull request #20049: feat(dashboard): Rearrange items in chart header controls dropdown

Posted by GitBox <gi...@apache.org>.

github-actions[bot] commented on PR #20049:
URL: https://github.com/apache/superset/pull/20049#issuecomment-1126268566

   Ephemeral environment shutdown and build artifacts deleted.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org