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

Let CSS do the uppercase transformation. #1521

Merged
merged 1 commit into from
Oct 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions arduino-ide-extension/src/browser/style/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ button.secondary[disabled], .theia-button.secondary[disabled] {
font-size: 14px;
}

.uppercase {
text-transform: uppercase;
}

/* High Contrast Theme rules */
/* TODO: Remove it when the Theia version is upgraded to 1.27.0 and use Theia APIs to implement it*/
Expand Down
2 changes: 0 additions & 2 deletions arduino-ide-extension/src/browser/style/list-widget.css
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,12 @@
max-height: calc(1em + 4px);
color: var(--theia-button-foreground);
content: attr(install);
text-transform: uppercase;
}

.component-list-item .header .installed:hover:before {
background-color: var(--theia-button-foreground);
color: var(--theia-button-background);
content: attr(uninstall);
text-transform: uppercase;
}

.component-list-item[min-width~="170px"] .footer {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ export class CloudSketchbookTreeWidget extends SketchbookTreeWidget {
</div>
</div>
<button
className="theia-button"
className="theia-button uppercase"
onClick={() => shell.openExternal('https://create.arduino.cc/editor')}
>
{nls.localize('cloud/GoToCloud', 'GO TO CLOUD')}
{nls.localize('arduino/cloud/goToCloud', 'Go to Cloud')}
Copy link
Contributor

Choose a reason for hiding this comment

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

Changing the localization key from 'cloud/GoToCloud' to 'arduino/cloud/goToCloud' is right, but what about all the translations of this string? Like this we would lose them all.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have also thought about the impact of prior changes to the keys. It was previously discussed here:

#1431 (comment)

I think there is an impact, but it can't quite be described as "lose them" because the translations are still stored in the "translation memory" on Transifex. So they don't need to be translated over again from English, but I don't think this happens automatically, meaning there is still an impact. I believe the translators will still need to select the strings with changed keys and then accept the auto-completed translation offered by Transifex.

Copy link
Contributor

Choose a reason for hiding this comment

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

So it seems to me that we should avoid frivolous key changes, but also shouldn't feel locked into the existing key names in cases where there is a significant benefit to restructuring them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is the perfect time to make such changes. I do not want to maintain a key that does make sense in the next two decades.

</button>
<div className="center item"></div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@ export class ListItemRenderer<T extends ArduinoComponent> {
)}
</span>
<span
className="installed"
className="installed uppercase"
onClick={onClickUninstall}
{...{
install: nls.localize('arduino/component/installed', 'INSTALLED'),
install: nls.localize('arduino/component/installed', 'Installed'),
uninstall: nls.localize('arduino/component/uninstall', 'Uninstall'),
}}
/>
Expand All @@ -77,10 +77,10 @@ export class ListItemRenderer<T extends ArduinoComponent> {
const onClickInstall = () => install(item);
const installButton = item.installable && (
<button
className="theia-button secondary install"
className="theia-button secondary install uppercase"
onClick={onClickInstall}
>
{nls.localize('arduino/component/install', 'INSTALL')}
{nls.localize('arduino/component/install', 'Install')}
</button>
);

Expand Down
8 changes: 3 additions & 5 deletions i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
"donePushing": "Done pushing ‘{0}’.",
"embed": "Embed:",
"emptySketchbook": "Your Sketchbook is empty",
"goToCloud": "Go to Cloud",
"learnMore": "Learn more",
"link": "Link:",
"notYetPulled": "Cannot push to Cloud. It is not yet pulled.",
Expand Down Expand Up @@ -144,8 +145,8 @@
"boardsIncluded": "Boards included in this package:",
"by": "by",
"filterSearch": "Filter your search...",
"install": "INSTALL",
"installed": "INSTALLED",
"install": "Install",
"installed": "Installed",
"moreInfo": "More info",
"uninstall": "Uninstall",
"uninstallMsg": "Do you want to uninstall {0}?",
Expand Down Expand Up @@ -422,9 +423,6 @@
"upload": "Upload"
}
},
"cloud": {
"GoToCloud": "GO TO CLOUD"
},
"theia": {
"core": {
"cannotConnectBackend": "Cannot connect to the backend.",
Expand Down