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

PublicDashboards: Configuration modal redesign #63211

Merged
merged 19 commits into from
Feb 24, 2023

Conversation

juanicabanas
Copy link
Contributor

@juanicabanas juanicabanas commented Feb 9, 2023

What is this feature?

Redesign for configuration modal

Which issue(s) does this PR fix?:

Fixes #4682

Special notes for your reviewer:

Before

pubdash-old-design.mov

After

pubdash-new-design.mov

@juanicabanas juanicabanas added this to the 9.5.0 milestone Feb 9, 2023
@juanicabanas juanicabanas requested a review from a team as a code owner February 9, 2023 17:03
@juanicabanas juanicabanas self-assigned this Feb 9, 2023
@juanicabanas juanicabanas requested review from a team as code owners February 9, 2023 17:03
@juanicabanas juanicabanas requested review from a team, axelavargas and kaydelaney and removed request for a team February 9, 2023 17:03
@juanicabanas juanicabanas requested review from ashharrison90, yaelleC, andresmgot, oscarkilhed and zoltanbedi and removed request for a team February 9, 2023 17:03
@polibb
Copy link
Contributor

polibb commented Feb 15, 2023

When I change the time range of the whole dashboard to something other than 6 hours, and go into sharing publicly, the sharing option is still on "Last 6 hours".

image

e2e().wait('@update');

// Url should be hidden
e2e.pages.ShareDashboardModal.PublicDashboard.CopyUrlInput().should('not.exist');
e2e.pages.ShareDashboardModal.PublicDashboard.CopyUrlInput().should('be.disabled');
e2e.pages.ShareDashboardModal.PublicDashboard.CopyUrlButton().should('be.disabled');
Copy link
Contributor

Choose a reason for hiding this comment

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

Great test coverage! 👏🏻

@juanicabanas
Copy link
Contributor Author

When I change the time range of the whole dashboard to something other than 6 hours, and go into sharing publicly, the sharing option is still on "Last 6 hours".

image

that's because you didn't save the current time range as dashboard default. Once you do it, the change will be shown

Copy link
Contributor

@kaydelaney kaydelaney left a comment

Choose a reason for hiding this comment

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

Looks good!

Extremely small nitpick, but I think some slight adjustments could be made to make everything perfectly center-aligned:

Before:
image

After (Notice the info icon):
image

If you'd like I could push the changes required for this (or if you think it looks better as it is now, that's totally fine too!)

@juanicabanas
Copy link
Contributor Author

d for this (or if you think it looks better as it is

sure! make the change yourself 💪🏽 thanks for the detail, looks quite better

@juanicabanas
Copy link
Contributor Author

@kaydelaney do you wanna make those changes?

@kaydelaney
Copy link
Contributor

@juanicabanas Yep! Will do it now

@kaydelaney kaydelaney requested a review from a team as a code owner February 23, 2023 13:30
@juanicabanas
Copy link
Contributor Author

@grafana/dataviz-squad could you take a look at Checkbox modifications? thanks!

@staton-hyse11
Copy link
Contributor

LGTM!

@juanicabanas juanicabanas merged commit 9df4a39 into main Feb 24, 2023
@juanicabanas juanicabanas deleted the juanicabanas/sharePublicDashboardRefactor branch February 24, 2023 15:36
ryantxu pushed a commit that referenced this pull request Mar 2, 2023
Configuration modal redesign

---------

Co-authored-by: kay delaney <kay@grafana.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants