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

Inconsistent Behavior for CRUD view modals #11520

Open
ktmud opened this issue Nov 1, 2020 · 9 comments
Open

Inconsistent Behavior for CRUD view modals #11520

ktmud opened this issue Nov 1, 2020 · 9 comments
Labels
#bug:cosmetic Cosmetic/layout/design tweak needed !deprecated-label:bug Deprecated label - Use #bug instead

Comments

@ktmud
Copy link
Member

ktmud commented Nov 1, 2020

Screenshot

dashboard-modal

Description

In the new React-based Dashboard & Chart CRUD view. The edit modal has animation on both opening and closing, but the edit modal has only opening animation.

Also, I think we should probably consider linking the edit button to opening <dashboard_url?>edit=true in a new window instead of opening the properties modal, because it's much more commonly needed to edit the dashboard layout/charts rather than just the things in properties modal.

Design input

[describe any input/collaboration you'd like from designers, and
tag accordingly. For design review, add the
label design:review. If this includes a design proposal,
include the label design:suggest]

@ktmud ktmud added the #bug:cosmetic Cosmetic/layout/design tweak needed label Nov 1, 2020
@issue-label-bot
Copy link

Issue-Label Bot is automatically applying the label #bug to this issue, with a confidence of 0.68. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

@issue-label-bot issue-label-bot bot added the !deprecated-label:bug Deprecated label - Use #bug instead label Nov 1, 2020
@ktmud
Copy link
Member Author

ktmud commented Nov 2, 2020

Cc @nytai

@nytai
Copy link
Member

nytai commented Nov 10, 2020

Let's have a conversation here (@junlincc @lilykuang @benceorlai @eschutho @mistercrunch ) about switching over the edit action on the dashboard list view.

Current the edit action only open the dashboard properties modal. Should we just make the call that "Editing a dashboard means opening the dashboard in edit mode and not opening the dashboard properties modal"? It seems that everyone I've talked to advocates for

linking the edit button to opening <dashboard_url?>edit=true in a new window instead of opening the properties modal, because it's much more commonly needed to edit the dashboard layout/charts rather than just the things in properties modal.

I think this should clear up a number of bugs/cosmetic issues around the current dashboard edit experience. Does anyone feel differently?

@ktmud
Copy link
Member Author

ktmud commented Nov 13, 2020

+1 for changing the edit button to open dashboard in edit mode.

@mistercrunch
Copy link
Member

+1 for changing the edit button to open dashboard in edit mode

Also! In dashboard edit mode, consolidate the 2-3 other modals (filter mapping, colors, auto-refresh) under other tabs under "Edit Dashboard Properties", and make that an Ok dialog (not Save), affect the local redux state, and be kept there until the dashboard's Save is hit.

@junlincc
Copy link
Member

junlincc commented Nov 13, 2020

+1 for changing the edit button to open dashboard in edit mode

Also! In dashboard edit mode, consolidate the 2-3 other modals (filter mapping, colors, auto-refresh) under other tabs under "Edit Dashboard Properties", and make that an Ok dialog (not Save), affect the local redux state, and be kept there until the dashboard's Save is hit.

Also Edit Chart Properties modal please~ 😉 @nytai

@nytai
Copy link
Member

nytai commented Nov 13, 2020

Looks like the portion about the strange saving behavior on the properties modal is covered by #11677

@rusackas
Copy link
Member

I want to close this issue since it's such an antique, but dang it... it's still true!

@eric-briscoe / @michael-s-molina / @geido if anyone has bandwidth, it'd be good to look at the modal component, and see if we can nail down the transitions globally, for the sake of the design system.

@jess-dillard / @kasiazjc if anyone has bandwidth, it might be worth (re)considering adding an additional action in the list view to let users edit the dashboard properties OR go right into dashboard edit mode. I'd contend that both have their place.

@kasiazjc
Copy link
Contributor

I want to close this issue since it's such an antique, but dang it... it's still true!

@eric-briscoe / @michael-s-molina / @geido if anyone has bandwidth, it'd be good to look at the modal component, and see if we can nail down the transitions globally, for the sake of the design system.

@jess-dillard / @kasiazjc if anyone has bandwidth, it might be worth (re)considering adding an additional action in the list view to let users edit the dashboard properties OR go right into dashboard edit mode. I'd contend that both have their place.

I think Karan is working on the CRUD views, I'll pass this one to him (not sure what is his github handle). Thanks @rusackas :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
#bug:cosmetic Cosmetic/layout/design tweak needed !deprecated-label:bug Deprecated label - Use #bug instead
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants