-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Improve dashboard editing flow #2219
Improve dashboard editing flow #2219
Conversation
I merged from the query editor branch to make sure we won't have any big conflicts later on... |
Ok, I merged the master here to ensure we won't have conflicts. I'm adding a couple of improvements and fixes. |
This is the final version. My only concern is the discoverability of the [Add Widget] button on a busy dashboard. I'm not worried about new users, since they'll learn when create their first dashboard and see it in empty form. But users who already have dashboard, we totally moved the option from the usual ... menu—let's see how it'll go, maybe we need to fine-tune and add back the ... in Edit mode. |
@arikfr is this a proper explanation for a widget: "Widgets are individual query visualisations you can place on your dashboard in various arrangements." how would you phrase it? |
Just a small change: "Widgets are individual query visualizations or text boxes you can place...". (visualizations btw, we use US spelling :-)) |
I created the single dashboard edit mode + open new dashboard in edit mode. While doing this notice a few things:
|
Thanks, Arik!
That's correct, I'll add an icon to open the modal (later we can implement the same click and edit on the title) -> no, actually let's keep it as it is for now so it's consistent across the app.
I'll make this consistent, thanks for spotting it. |
There is already an edit in place. |
…esign improvements
…it's very messy here)
91b1504
to
4029821
Compare
margin-top: 15px; | ||
//box-shadow: fade(@redash-gray, 15%) 0px 4px 9px -3px; | ||
border: 2px dashed fade(@redash-gray, 25%); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kocsmy I'm not sure if this is really not needed, but after rebasing with latest master this part was making the background grey & transparent (you could see the footer behind the add-widget section). I merged the edit flow PR, but please review the CSS in the final state to make sure the rebase didn't mess up something...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for removing that part, it was indeed just a leftover from previous iterations. I'll review the CSS to make sure everything is in place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I reviewed it and all seems good now. 👌
#2213
(until #2185 is merged this pull request contains commits from that branch too)