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

ui: make Metrics and SQL timepicker align #83107

Merged
merged 1 commit into from
Jun 27, 2022

Conversation

maryliag
Copy link
Contributor

@maryliag maryliag commented Jun 20, 2022

Previously, the timepicker from Metrics page and
the timepicker on SQL Activity pages acted independently.
Now, if the value of one changes, the other value changes
to the same period selected.

This commit also fixes a bug where the period selected
would change to a custom value if the Metrics page was
refreshed.

Fixes #78187
Fixes #82152

Release note (ui change): The period selected on the Metrics
page and the SQL Activity pages are now aligned. If the user
changes in one page, the value will be the same for the other.

Release note (bug fix): The period selected on Metrics page
continues the same when refreshing the page, no longer changing
to a custom period.

@maryliag maryliag requested a review from a team June 20, 2022 19:33
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Member

@xinhaoz xinhaoz left a comment

Choose a reason for hiding this comment

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

Reviewed 11 of 16 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @maryliag and @xinhaoz)


pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/utils.ts line 87 at r1 (raw file):

    : moment().utc();
  const endRounded = end.set({ millisecond: 0 });
  const start = moment.utc(endRounded).subtract(ts.windowSize);

Do we have unit tests for this function? Maybe we should consider adding them.

@maryliag maryliag force-pushed the global-timepicker branch 6 times, most recently from 03bc28a to 9718525 Compare June 24, 2022 18:11
Copy link
Contributor Author

@maryliag maryliag left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @xinhaoz)


pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/utils.ts line 87 at r1 (raw file):

Previously, xinhaoz (Xin Hao Zhang) wrote…

Do we have unit tests for this function? Maybe we should consider adding them.

Test added

@maryliag maryliag force-pushed the global-timepicker branch from 9718525 to e18d72f Compare June 24, 2022 18:15
Previously, the timepicker from Metrics page and
the timepicker on SQL Activity pages acted independently.
Now, if the value of one changes, the other value changes
to the same period selected.

This commit also fixes a bug where the period selected
would change to a custom value if the Metrics page was
refreshed.

Fixes cockroachdb#78187
Fixes cockroachdb#82152

Release note (ui change): The period selected on the Metrics
page and the SQL Activity pages are now aligned. If the user
changes in one page, the value will be the same for the other.

Release note (bug fix): The period selected on Metrics time picker
continues the same when refreshing the page, no longer changing
to a custom period.
@maryliag maryliag force-pushed the global-timepicker branch from e18d72f to 78b723d Compare June 24, 2022 19:16
Copy link
Member

@xinhaoz xinhaoz left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 16 files at r1, 3 of 6 files at r2.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @xinhaoz)

@maryliag
Copy link
Contributor Author

TFTR!
bors r+

@craig craig bot merged commit 9dfdf1b into cockroachdb:master Jun 27, 2022
@craig
Copy link
Contributor

craig bot commented Jun 27, 2022

Build succeeded:

@blathers-crl
Copy link

blathers-crl bot commented Jun 27, 2022

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.


error creating merge commit from 78b723d to blathers/backport-release-22.1-83107: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 22.1.x failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants