feat(explore): Move chart header to top of the page #19529

kgabryje · 2022-04-05T09:37:09Z

SUMMARY

This PR implements part of chart header redesign (final design available here: #19099)

List of changes:

move chart header to top of the page
change looks and behaviour of editable title - input grows when user types text, no wrapping, when title overflows it gets truncated and tooltip with full name appears on hover
when title is truncated, clicking (enabling editing mode) moves the cursor to the end of the title
Applied additional paddings to Dataset label and column search on the left so that everything is aligned
Deleting the whole title is allowed - we don't revert to the last title when user clears the input. Instead, a placeholder is shown
When new chart is created, the default title is empty instead of "- untitled", which means that we display the placeholder
Editing the name of existing chart gets recorded in "Altered" table
if user doesn't have edit permissions, instead of an input element display span element that looks and behaves the same except for editing capabilities

Next steps:

move timer, rows retrieved and cached labels to chart container (implemented in feat(explore): Move timer, row counter and cached pills to chart container #19458)
move save button to the right of chart header

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen.Recording.2022-04-05.at.11.35.42.mov

TESTING INSTRUCTIONS

Open a chart that you have permissions to edit
Click the title, verify that cursor appears
When there's no text a placeholder should appear
When you start typing, the input's width should adjust to its content
When input is wider than the header container, it should move to the position of cursor if in edit mode, or be truncated if not in edit mode
If not editing and title is truncated, display tooltip with full title on hover

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
CC @kasiazjc

kgabryje · 2022-04-05T10:19:45Z

superset-frontend/src/explore/components/ExploreChartHeader/ChartEditableTitle/index.tsx

+          </span>
+        )}
+      </Tooltip>
+      <span ref={sizerRef} className="input-sizer" aria-hidden tabIndex={-1} />


That span is invisible, we need it to measure text width in order to scale input's width dynamically

kgabryje · 2022-04-05T10:34:53Z

/testenv up

github-actions · 2022-04-05T10:36:57Z

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

villebro

Approving with a minor nit (not important): Very clean and I like this much better than the current UX. A few thoughts that came to mind when testing: it's not obvious to the user that the title has been edited. I believe some time ago it used to save it when pressing return? I'm not saying that was very intuitive, either, but since we have the pill that says when the form data has changed, maybe we should also record the changed title in it?

But let's leave that for a follow up, as it's not really directly related to this PR.

villebro · 2022-04-05T11:03:47Z

superset-frontend/src/explore/components/ExploreChartHeader/index.jsx

+            title={this.getSliceName()}
+            canEdit={
+              !slice ||
+              this.props.can_overwrite ||


nit: while we're using the destructured slice, could we also destructure this like the others on line 224: const { ..., can_overwrite: canOverwrite } = this.props. Also maybe this.props.actions.updateChartTitle

codecov · 2022-04-05T11:26:58Z

Codecov Report

Merging #19529 (0559f67) into master (90dbe8d) will increase coverage by 0.00%.
The diff coverage is 72.72%.

@@           Coverage Diff           @@
##           master   #19529   +/-   ##
=======================================
  Coverage   66.58%   66.59%           
=======================================
  Files        1677     1681    +4     
  Lines       64238    64279   +41     
  Branches     6538     6553   +15     
=======================================
+ Hits        42773    42804   +31     
- Misses      19766    19775    +9     
- Partials     1699     1700    +1

Flag	Coverage Δ
javascript	`51.35% <72.72%> (+0.03%)`	⬆️

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

Impacted Files	Coverage Δ
...d/src/explore/components/DatasourcePanel/index.tsx	`69.23% <ø> (ø)`
.../explore/components/ExploreViewContainer/index.jsx	`54.39% <42.85%> (-2.60%)`	⬇️
...rc/explore/components/ExploreChartHeader/index.jsx	`46.03% <75.00%> (-3.97%)`	⬇️
...ts/ExploreChartHeader/ChartEditableTitle/index.tsx	`78.43% <78.43%> (ø)`
...uperset-frontend/src/components/FaveStar/index.tsx	`100.00% <100.00%> (ø)`
...ntend/src/explore/components/ExploreChartPanel.jsx	`74.24% <100.00%> (+2.24%)`	⬆️
...re/components/controls/DatasourceControl/index.jsx	`72.72% <100.00%> (+0.35%)`	⬆️
...t-frontend/src/components/AsyncAceEditor/index.tsx	`90.90% <0.00%> (-0.21%)`	⬇️
superset-frontend/src/SqlLab/App.jsx	`0.00% <0.00%> (ø)`
...ntend/src/SqlLab/components/QueryHistory/index.tsx	`66.66% <0.00%> (ø)`
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 90dbe8d...0559f67. Read the comment docs.

kgabryje · 2022-04-05T12:18:23Z

I believe some time ago it used to save it when pressing return?

It still does 🙂 I like the idea of including the title in altered table! I'll add that to this PR

kgabryje · 2022-04-05T12:31:08Z

@villebro

github-actions · 2022-04-05T13:20:53Z

Ephemeral environment shutdown and build artifacts deleted.

* Move chart header to top of the page * Implement truncating and dynamic input * fix typing * Prevent cmd+z undoing changes when not in edit mode * Fix tests, add missing types * Show changed title in altered

kgabryje added 2 commits April 4, 2022 12:53

Move chart header to top of the page

73b16c7

Implement truncating and dynamic input

1f830c1

pull-request-size bot added the size/XL label Apr 5, 2022

kgabryje requested review from geido, michael-s-molina and villebro April 5, 2022 09:47

kgabryje added 2 commits April 5, 2022 11:51

fix typing

c38e0a8

Prevent cmd+z undoing changes when not in edit mode

17098fa

kgabryje commented Apr 5, 2022

View reviewed changes

Fix tests, add missing types

31e3a28

villebro approved these changes Apr 5, 2022

View reviewed changes

Show changed title in altered

0559f67

kgabryje merged commit 602afba into apache:master Apr 5, 2022

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(explore): Move chart header to top of the page #19529

feat(explore): Move chart header to top of the page #19529

kgabryje commented Apr 5, 2022 •

edited

Loading

kgabryje Apr 5, 2022 •

edited

Loading

kgabryje commented Apr 5, 2022

github-actions bot commented Apr 5, 2022

villebro left a comment

villebro Apr 5, 2022

kgabryje Apr 5, 2022

codecov bot commented Apr 5, 2022 •

edited

Loading

kgabryje commented Apr 5, 2022

kgabryje commented Apr 5, 2022

github-actions bot commented Apr 5, 2022

feat(explore): Move chart header to top of the page #19529

feat(explore): Move chart header to top of the page #19529

Conversation

kgabryje commented Apr 5, 2022 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

kgabryje Apr 5, 2022 • edited Loading

Choose a reason for hiding this comment

kgabryje commented Apr 5, 2022

github-actions bot commented Apr 5, 2022

villebro left a comment

Choose a reason for hiding this comment

villebro Apr 5, 2022

Choose a reason for hiding this comment

kgabryje Apr 5, 2022

Choose a reason for hiding this comment

codecov bot commented Apr 5, 2022 • edited Loading

Codecov Report

kgabryje commented Apr 5, 2022

kgabryje commented Apr 5, 2022

github-actions bot commented Apr 5, 2022

kgabryje commented Apr 5, 2022 •

edited

Loading

kgabryje Apr 5, 2022 •

edited

Loading

codecov bot commented Apr 5, 2022 •

edited

Loading