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: Fixes for various small UI bugs #14007

Merged
merged 1 commit into from
Mar 9, 2017

Conversation

mrtracy
Copy link
Contributor

@mrtracy mrtracy commented Mar 9, 2017

A grab bag of various micro-fixes, each closing a github issue.

Fixes #12658

The custom time range displayed in the top bar on the cluster overview page is
currently displaying times in the local time zone, which does not match the
ticks on graphs. This change converts it to display in UTC.

Fixes #12997

Disables the "searchable" option on our dropdowns to prevent the blinking-cursor
effect. This option is not necessary for dropdowns at our current cluster sizes,
and the "search" mode of dropdowns has not been styled, so we simply disable to
search option.

Fixes #13002

Axis on "Capacity" graph is now properly labeled with Byte units.


This change is Reviewable

@tamird
Copy link
Contributor

tamird commented Mar 9, 2017

typo in commit message "disable to search option" - i think you meant "the".

:lgtm:


Reviewed 4 of 4 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


pkg/ui/app/containers/timescale.tsx, line 28 at r1 (raw file):

class TimeRange extends React.Component<TimeRangeProps, {}> {
  render() {
    let s = this.props.start.clone().utc();

why does this need to clone?


Comments from Reviewable

A grab bag of various micro-fixes, each closing a github issue.

Fixes cockroachdb#12658

The custom time range displayed in the top bar on the cluster overview page is
currently displaying times in the local time zone, which does not match the
ticks on graphs. This change converts it to display in UTC.

Fixes cockroachdb#12997

Disables the "searchable" option on our dropdowns to prevent the blinking-cursor
effect. This option is not necessary for dropdowns at our current cluster sizes,
and the "search" mode of dropdowns has not been styled, so we simply disable the
search option.

Fixes cockroachdb#13002

Axis on "Capacity" graph is now properly labeled with Byte units.
@mrtracy mrtracy force-pushed the mtracy/timespan_utc branch from a4f25c2 to 4356ef3 Compare March 9, 2017 03:03
@mrtracy
Copy link
Contributor Author

mrtracy commented Mar 9, 2017

Fixed the commit message typo.


Review status: all files reviewed at latest revision, 1 unresolved discussion.


pkg/ui/app/containers/timescale.tsx, line 28 at r1 (raw file):

Previously, tamird (Tamir Duberstein) wrote…

why does this need to clone?

Because moment.js is a fairly outdated library, and utc(), or any function that changes the display timezone, and in fact most methods in this library mutate the original object.

The have been aware of this since 2014: moment/moment#1754

They are hesitant to make it immutable because it would so radically break backwards compatibility; however, nobody else has stepped up to the plate and created an immutable version.

As of now, using "clone()" all over the place is the only way to get sane behavior.


Comments from Reviewable

@mrtracy mrtracy merged commit 3674a3a into cockroachdb:master Mar 9, 2017
@mrtracy mrtracy deleted the mtracy/timespan_utc branch March 9, 2017 06:04
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