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

Added space between temperature reading and unit #429

Merged
merged 5 commits into from
Mar 7, 2017

Conversation

barisi
Copy link
Contributor

@barisi barisi commented Mar 3, 2017

By submitting this pull request, I confirm the following (please check boxes, eg [X] - no spaces) Failure to fill the template will close your PR:

Please submit all pull requests against the development branch. Failure to do so will delay or deny your request

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

How familiar are you with the codebase?:

3


_Updated the rendering of temperature readings to include a space between the reading and the unit as suggested in issue #422 _

This template was created based on the work of udemy-dl.

@AzureMarker
Copy link
Contributor

AzureMarker commented Mar 4, 2017

Approved

Approved with PullApprove

@DL6ER
Copy link
Member

DL6ER commented Mar 4, 2017

Approved

Approved with PullApprove

@Mcat12 will have to re-approve this PR, since you made changes since the last time he approved your code.

@ProtoFoo
Copy link

ProtoFoo commented Mar 5, 2017

Sorry to be nitpicking about this, but if you add a non-breaking space before the temperature unit, please also add one before the percent sign at the end of line 351 in header.php.

E. g. Memory usage: 7.2% --> Memory usage: 7.2 %

@DL6ER
Copy link
Member

DL6ER commented Mar 5, 2017

English style guides prescribe writing the number and percent sign without any space between:

percentage rises
seem to give us a lot of problems: an increase from 3% to 5% is a 2 percentage point increase or a 2-point increase, not a 2% increase; any sentence saying “such and such rose or fell by X%” should be considered and checked carefully

See e.g. here.

However, the SI handbook in fact describes the percentage sign % as a mere unit for expression dimensionless quantities. In fact, they explicitly describe to add the space:

In mathematical expressions, the internationally recognized symbol % (percent) may be used with the SI to represent the number 0.01. Thus, it can be used to express the values of dimensionless quantities. When it is used, a space separates the number and the symbol %. In expressing the values of dimensionless quantities in this way, the symbol % should be used rather than the name "percent".

See e.g. here.

In the end, it might be better to stick to the SI recommendations. How about finding a middle way and use a thin space in between the number and the percentage? Might also be worth to test this for the units:  . This is also what I'm doing in my reports (LaTeX-only user).

@barisi
Copy link
Contributor Author

barisi commented Mar 5, 2017

I've updated the code to have   between the memory utilisation reading and %. It renders the same way as  

@DL6ER
Copy link
Member

DL6ER commented Mar 5, 2017

Approved

Approved with PullApprove

@ProtoFoo
Copy link

ProtoFoo commented Mar 5, 2017

Nice solution. Thanks guys! This is how it looks in Firefox 51.0.1:
image

@AzureMarker
Copy link
Contributor

AzureMarker commented Mar 7, 2017

Approved

Approved with PullApprove

@AzureMarker AzureMarker merged commit 1b64678 into pi-hole:devel Mar 7, 2017
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.

4 participants