Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

style: use tabs in dashboard edit pane #10394

Merged
merged 10 commits into from
Aug 6, 2020

Conversation

mistercrunch
Copy link
Member

@mistercrunch mistercrunch commented Jul 22, 2020

Currently the east pane in the "edit dashboard" flow is really confusing. The "Components" and "Colors" buttons at the top really act as tabs, and the "slider" pattern is kind of cool but confusing.

after (draft - general direction)

Screen Shot 2020-07-23 at 11 47 55 PM

Screen Shot 2020-07-23 at 11 47 38 PM

before

Screen Shot 2020-07-22 at 9 20 16 AM

  • just noticed that Colors could be moved to properties too - maybe

@Steejay
Copy link

Steejay commented Jul 22, 2020

@mistercrunch wondering if we should change order to charts>components>colors? Charts are the main element of a dashboard.

@mistercrunch
Copy link
Member Author

I have not usage data to back this up, but for me I always "Save and add to dashboard" from explore and almost never use the chart panel in the "dashboard editor"

@ktmud
Copy link
Member

ktmud commented Jul 27, 2020

Never even noticed the "Colors" setting. Agreed this is weird.

I use the Charts panel a lot. Sometimes you want to relayout the dashboard and it's easier to delete a chart first then re-add it later. Wondering if we can just keep the "Insert Components" panel as is and move "Set Color Scheme" to a modal triggered by a link in the dropdown menu?

image

Or we can change "Edit CSS" to "Change Appearance" and move color scheme selector to the Edit CSS modal. The same modal may even hold a "Select Theme" control in the future.

@mistercrunch mistercrunch marked this pull request as ready for review July 28, 2020 05:54
<span // eslint-disable-next-line react/no-danger
dangerouslySetInnerHTML={{ __html: datasourceLink }}
/>
<a href={datasourceUrl}>{datasourceName}</a>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bycatch - moving away from dangerouslySetInnerHTML

@mistercrunch
Copy link
Member Author

@ktmud I did a lot of work to untangle the sliders and move towards tabs as defined in SIP-34 here, I'd hate to roll this back.
Screen Shot 2020-07-28 at 5 04 28 PM

@ktmud
Copy link
Member

ktmud commented Jul 29, 2020

Makes sense. I can see why it'd be useful to have the tabs.

Here's another screenshot of SIP-34 design with proper tab titles and icons:

@mistercrunch
Copy link
Member Author

Oh I've been using Figma and I think something went wrong in the Sketch -> Figma transition in this area ... @Steejay ^^^

@mistercrunch mistercrunch force-pushed the edit-dash-imp branch 2 times, most recently from a3bed12 to 1043aef Compare August 4, 2020 23:09
Copy link
Member

@suddjian suddjian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mistercrunch mistercrunch merged commit ece9192 into apache:master Aug 6, 2020
@mistercrunch mistercrunch deleted the edit-dash-imp branch August 6, 2020 01:53
Ofeknielsen pushed a commit to ofekisr/incubator-superset that referenced this pull request Oct 5, 2020
* style: use tabs in dashboard edit pane

* fix tests

* more hackin'

* getting ready to rip cell measurer

* working

* pogress

* Fix cards

* done

* fix jest

* fix cy
auxten pushed a commit to auxten/incubator-superset that referenced this pull request Nov 20, 2020
* style: use tabs in dashboard edit pane

* fix tests

* more hackin'

* getting ready to rip cell measurer

* working

* pogress

* Fix cards

* done

* fix jest

* fix cy
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.38.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/XXL 🚢 0.38.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants