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

Gauge polish #14194

Merged
merged 2 commits into from Jun 25, 2019
Merged

Gauge polish #14194

merged 2 commits into from Jun 25, 2019

Conversation

chnn
Copy link
Contributor

@chnn chnn commented Jun 24, 2019

I investigated #14175 + https://github.com/influxdata/EAR/issues/823 and discovered the root issue is that the threshold settings component emits nonsensical settings. I opened up #14190 to track resolution.

In the meantime I made some minor gauge tweaks. It now displays better on HiDPI / Retina screens.

Before (you will need a HiDPI screen to see the blurriness):

before

After:

after

I also tweaked how the gauge behaves when the needle position is beyond the domain of the gauge (for example if the current value is 6,000,000 and the max value for the gauge is 100). Before the needle would display at an essentially random position ¯_(ツ)_/¯ (some sort of modular arithmetic).

Now it will be clamped to just beyond the domain:

Screen Shot 2019-06-24 at 3 30 58 PM

Copy link
Contributor

@ischolten ischolten left a comment

Choose a reason for hiding this comment

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

Looks great to me! Just one suggestion.

TBH i didnt notice the blurriness until this pr and my mind is blown!! It looks so much better now!
GREAT JOB

const {width, height} = this.props
const dpRatio = window.devicePixelRatio || 1

// Set up canvas to draw on HiDPI / Retina screens correctly
Copy link
Contributor

Choose a reason for hiding this comment

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

wooot!

@@ -282,15 +282,14 @@ class Gauge extends Component<Props> {
const {prefix, suffix, decimalPlaces} = this.props
const {degree, lineCount, labelColor, labelFontSize} = GAUGE_SPECS

const incrementValue = (maxValue - minValue) / lineCount
const tickValues = [
..._.range(minValue, maxValue, Math.abs(maxValue - minValue) / lineCount),
Copy link
Contributor

Choose a reason for hiding this comment

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

🚀


const labels = []
for (let g = minValue; g < maxValue; g += incrementValue) {
Copy link
Contributor

Choose a reason for hiding this comment

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

🔥 🔥 🔥

const needleRotation = (gaugePosition - minValue) / (maxValue - minValue)
let needleRotation: number

if (gaugePosition <= minValue) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is awesome! how do you feel about a function like getNeedleRotation and sticking that logic there?

@chnn chnn merged commit 530ceb9 into master Jun 25, 2019
@chnn chnn deleted the ui-fix-gauges branch June 25, 2019 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants