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

feat: Add option to use imperial units when doing unit conversions #1836

Merged
merged 3 commits into from
Jun 24, 2024

Conversation

StarScape
Copy link
Contributor

@StarScape StarScape commented Jun 22, 2024

I will be the first person to admit that the imperial system is dumb, but it's what most Americans are accustomed to, and unfortunately I don't have an intuitive sense of how big a square meter is any more than I do 1畳, so hopefully this will be helpful to other SI-challenged folks like myself :).

It will still default to metric unless you set the setting to imperial.

@birtles and @SaltfishAmi, could use your help for the Japanese and Chinese l10ns if you don't mind (though in practice, I really doubt anybody will be using this in Chinese). Strings I need are:

"metric" — "メートル法" for Japanese?

"imperial" - "帝国単位" for Japanese?

"Unit conversion" (heading for the setting section with the drop down to select metric/imperial)

"Preferred units" (label on the drop down to select metric/imperial preference)

@SaltfishAmi
Copy link
Contributor

I would be absolutely astonished if anyone using the Chinese locale ever needs this function, but here they are:

"metric" = 公制 (lit. "common system")

"imperial" = 英制 (lit. "British system" - I know that Brits have switched to metric system, sorry, but you guys started this mess at the first place, so not very much sorry)

"Unit conversion" = 单位转换

I don't really understand what does "label on the drop down to select metric/imperial preference" mean, can you provide a mock image for this?

BTW, having lived in the US for two years I still have zero idea about how big a sqft is.

@StarScape
Copy link
Contributor Author

Thanks!

I don't really understand what does "label on the drop down to select metric/imperial preference" mean, can you provide a mock image for this?

Yeah, I probably should have included an image to begin with. Here's the new section in the settings menu:

image

The label to the left of the drop-down menu is what I'm after.

I would be absolutely astonished if anyone using the Chinese locale ever needs this function

Me too :). But I imagine it would still be jarring to see part of the UI in English if everything else is Chinese.

BTW, having lived in the US for two years I still have zero idea about how big a sqft is.

I think most people's point of reference for it is the size of typical floorplans. Very small 1 room apartment ≈ 100 sq ft ≈ 10 m2. Typical 2 bedroom, spacious apartment with a kitchen and living room ≈ 1000 sq ft ≈ 100 m2.

@SaltfishAmi
Copy link
Contributor

Oh. In this case you can translate "Preferred units" to simply 单位制 (lit. unit systems) which omits "preferred" but user can infer it anyway.

If you want a more accurate translation, the nature order of words in Chinese would be prefer (dropdown menu) units which adds more complexity to the ui implementation and I don't think its worth it.

I live in a studio and I don't even know how many sqfts it has lol

@StarScape
Copy link
Contributor Author

If you want a more accurate translation, the nature order of words in Chinese would be prefer (dropdown menu) units which adds more complexity to the ui implementation and I don't think its worth it.

Yeah, agreed. Thanks!

Copy link
Member

@birtles birtles left a comment

Choose a reason for hiding this comment

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

As best I can tell

"metric" — "メートル法" for Japanese?

Yes, sounds good!

"imperial" - "帝国単位" for Japanese?

As best I can tell, ヤード・ポンド法 might be more common?

"Unit conversion" (heading for the setting section with the drop down to select metric/imperial)

単位換算 I think

"Preferred units" (label on the drop down to select metric/imperial preference)

Like the Chinese, maybe just 単位系 is fine here.

_locales/en/messages.json Outdated Show resolved Hide resolved
_locales/en/messages.json Outdated Show resolved Hide resolved
Comment on lines +406 to +407
// UPDATE: Looks like support was added for decorators as of esbuild v0.21.3
// https://github.com/evanw/esbuild/releases/tag/v0.21.3
Copy link
Member

Choose a reason for hiding this comment

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

Yes, I saw that! That looks very promising.

I notice that the latest vitest doesn't cover that version of esbuild yet: https://github.com/vitest-dev/vitest/blob/6b29f3ddc86060cf3265959d4ae32e90b186cb92/package.json#L53

Perhaps there is a way via yarn resolutions to force it to use the latest esbuild, however?

It sure would be nice to start dropping all this boilerplate. It's so much work just to add a pref at the moment as I'm sure you discovered!

src/common/config.ts Outdated Show resolved Hide resolved
src/common/config.ts Outdated Show resolved Hide resolved
src/content/content-config.ts Outdated Show resolved Hide resolved
src/content/content.ts Outdated Show resolved Hide resolved
value: number;
alt?: Array<AlternateMeasure>;
};

export type AlternateMeasure = {
type: 'kyouma' | 'chuukyouma' | 'edoma' | 'danchima';
label?: string;
unit: '畳' | 'm2';
unit: '畳' | 'm2' | 'sq ft';
Copy link
Member

Choose a reason for hiding this comment

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

Just a thought, in the Japanese localization, do we still want to display "sq ft" or "平方フィート" or even ft²?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This crossed my mind, but I think this is Americentric enough that displaying it in the Latin alphabet is reasonable even in Japanese. If I'm not mistaken, the Japanese localization still displays m², right, rather than 平方メートル? So I think ft², for consistency.

src/content/popup/show-popup.ts Outdated Show resolved Hide resolved
meta: MeasureMeta,
preferredUnits: ContentConfigParams['preferredUnits']
): HTMLElement {
const converted = convertMeasure(meta, preferredUnits);

const metaDiv = html(
Copy link
Member

Choose a reason for hiding this comment

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

Currently, this calls renderUnit but doesn't set showRuby to false so it renders like this for me (with じょう in furigana above "sq ft"):

image

Copy link
Contributor

Choose a reason for hiding this comment

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

これを見て噴き出したwww
sq ftの上に「へいほうふぃーと」ってルビ付けるのも有りかも?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lol, thanks for catching this. I've added a special case for sq ft that renders it as ft², which should get rid of the issue.

- Alphabetically sort `preferredUnits` in all settings/config lists
- Fix bug where ruby じょう was rendered above 'sq ft' and display it as
  ft² instead
- Capitalized imperial/metric in drop-down
- Add in new badge for in settings for unit conversion
@StarScape
Copy link
Contributor Author

StarScape commented Jun 22, 2024

BTW @birtles, I know this is your actual day job, so don’t feel you’re obligated to respond on the weekend if you don’t want to. Though if you do want to, of course I won’t stop you 🙂.

@birtles birtles merged commit a21b92c into birchill:main Jun 24, 2024
2 checks passed
@birtles
Copy link
Member

birtles commented Jun 24, 2024

Thanks so much @StarScape!

I've merged this now. One thing I don't think we covered, was converting from square feet. For example, for square metres we convert to 帖 like so:

image

I don't remember exactly why we implemented that—possibly for when you're house hunting and you can more easily compare between listings in 帖 vs square metres. I'm not sure how useful it would be for square feet and it might be more tricky to match "sq ft" and "sq. ft." that it is to match m2/㎡.

@StarScape
Copy link
Contributor Author

StarScape commented Jun 24, 2024

Yeah, I did notice that the conversions from m2 were there. I opted to leave out the conversions from sq ft to 帖/畳 for basically the reasons you mentioned—I wasn't sure it would be that useful, and "sq ft" is commonly written in a lot more ways than square meters are (sq ft, sq. ft., square feet, sqft, sf, and ft² are all pretty common in my experience), which would complicate the implementation more than is probably worth it.

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

3 participants