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

CLDR-17080 Regional Variants menu follow-up #3372

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

btangmu
Copy link
Member

@btangmu btangmu commented Oct 31, 2023

-Set SIDEWAYS_DEBUG to false

-Remove the 2-second delay if (and only if) the user has just chosen from the menu

-Shorten excessively long functions by moving nested functions to top level and making more subroutines

-Remove dependency of the Vue component on non-GUi modules cldrLoad, cldrStatus

-Refactor for clarity

-Comments

CLDR-17080

  • This PR completes the ticket.

ALLOW_MANY_COMMITS=true

-Set SIDEWAYS_DEBUG to false

-Remove the 2-second delay if (and only if) the user has just chosen from the menu

-Shorten excessively long functions by moving nested functions to top level and making more subroutines

-Refactor for clarity

-Comments
@btangmu btangmu self-assigned this Oct 31, 2023
@btangmu btangmu marked this pull request as ready for review October 31, 2023 16:49
@btangmu btangmu requested a review from srl295 October 31, 2023 17:06
@@ -8,27 +8,48 @@ import * as cldrLoad from "./cldrLoad.mjs";
import * as cldrStatus from "./cldrStatus.mjs";
import * as cldrSurvey from "./cldrSurvey.mjs";

const SIDEWAYS_DEBUG = true;
const SIDEWAYS_DEBUG = false;
Copy link
Member

Choose a reason for hiding this comment

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

yeah, this should be a .gitignored variable setting… 

You know what we could do is have a separate .mjs file just for debugging settings. And then, you could use git skip to keep that file out of the PR.

Then in this file:

import * as cldrDebug from "./cldrDebug.mjs";

const SIDEWAYS_DEBUG = cldrDebug.get("SIDEWAYS_DEBUG");

then you could do local tweaks without having to commit them.

Copy link
Member Author

Choose a reason for hiding this comment

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

that's worth thinking about...

Comment on lines +13 to +14
const NON_BREAKING_SPACES = "\u00A0\u00A0\u00A0"; // non-breaking spaces
const UNEQUALS_SIGN = "\u2260\u00A0"; // U+2260 = "≠"
Copy link
Member

Choose a reason for hiding this comment

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

perhaps this should go in cldrText? or in another common place

* json.novalue, by assigning a value that's different from curValue.
*/
if (json.novalue) {
const differentValue = curValue === "A" ? "B" : "A"; // anything different from curValue
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const differentValue = curValue === "A" ? "B" : "A"; // anything different from curValue
const differentValue = NaN;

It'll never be === any other string. Or it could even be const differentvalue = Symbol("no value");

const popupSelect = {
items: [],
label:
"Regional Variants for " + locmap.getRegionAndOrVariantName(topLocale),
Copy link
Member

Choose a reason for hiding this comment

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

ideally cldrText

@btangmu btangmu merged commit f502341 into unicode-org:main Nov 7, 2023
8 checks passed
@btangmu btangmu deleted the t17080_i branch November 7, 2023 17:24
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.

2 participants