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

Update Dashboard and Alert headers with the Query one #4710

Merged
merged 5 commits into from
Mar 6, 2020

Conversation

gabrieldutra
Copy link
Member

What type of PR is this? (check all applicable)

  • Refactor

Description

Following #4694, this PR updates Dashboard and Alert headers to behave similar to the Query Page one (e.g: wrapping the controls)

Related Tickets & Documents

#2796

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

Dashboard Page

Before After
dashboard-header-before dashboard-header-after

Alert Page

Before After
alert-header-before alert-header-after

@gabrieldutra gabrieldutra self-assigned this Mar 4, 2020
Comment on lines -47 to -53
.alert-actions {
flex-grow: 1;
display: flex;
justify-content: flex-end;
align-items: center;
margin-right: -15px;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

From this file I just moved this to Title.less, the rest is on Prettier.

Comment on lines -125 to -141
.page-header--query {
.page-title {
display: block;
}

.tags-control a {
opacity: 0;
transition: opacity 0.2s ease-in-out;
}

&:hover {
.tags-control a {
opacity: 1;
}
}
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Not used anymore.

Comment on lines -521 to -528
// Responsive fixes
@media (max-width: 767px) {
.query-page-wrapper {
h3 {
font-size: 18px;
}
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved to QueryPageHeader.less

Comment on lines +198 to +209
{showShareButton && (
<Tooltip title="Dashboard Sharing Options">
<Button
className="icon-button m-l-5"
type={buttonType(dashboard.publicAccessEnabled)}
onClick={showShareDashboardDialog}
data-test="OpenShareForm">
<i className="zmdi zmdi-share" />
</Button>
</Tooltip>
)}
{showMoreOptionsButton && <DashboardMoreOptionsButton dashboardOptions={dashboardOptions} />}
Copy link
Member Author

Choose a reason for hiding this comment

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

Now that the controls are wrapped for mobiles, I'm not hiding those buttons anymore (I saw no reason to keep doing it)

Copy link
Member

Choose a reason for hiding this comment

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

Let's continue hiding the full screen button as it's meaningless on mobile.

Copy link
Member Author

Choose a reason for hiding this comment

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

That one is still hidden, only the Share and the Dropdown were changes.

margin: 0;

@media (max-width: 767px) {
font-size: 18px;
Copy link
Member Author

Choose a reason for hiding this comment

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

Copied this behavior from the Query Page to have more room for tags.

@@ -2,7 +2,7 @@
display: flex;
flex-wrap: wrap;
align-items: stretch;
margin: -5px 0;
margin-bottom: -5px;
Copy link
Member Author

Choose a reason for hiding this comment

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

This change keeps a margin on the top of the Query Page (to be more consistent with other pages).

@gabrieldutra gabrieldutra merged commit bf3095c into master Mar 6, 2020
@gabrieldutra gabrieldutra deleted the update-headers branch March 6, 2020 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants