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 Add Regional Variants label for the Info Panel menu #3361

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

btangmu
Copy link
Member

@btangmu btangmu commented Oct 24, 2023

-New component InfoRegionalVariants.vue; related refactoring, modernization

-Fix bug, erase deferred/placeholder help when click Next/Previous

-Remove dead code

CLDR-17080

  • This PR completes the ticket.

ALLOW_MANY_COMMITS=true

-New component InfoRegionalVariants.vue; related refactoring, modernization

-Fix bug, erase deferred/placeholder help when click Next/Previous

-Remove dead code
@btangmu btangmu self-assigned this Oct 24, 2023
@btangmu btangmu marked this pull request as ready for review October 24, 2023 17:34
@btangmu btangmu requested a review from srl295 October 24, 2023 17:34
@btangmu
Copy link
Member Author

btangmu commented Oct 24, 2023

This PR essentially completes the ticket, however I'd like to follow up with some further improvements:

  • Set SIDEWAYS_DEBUG to false
  • Remove the 2-second delay if (and only if) the user has just chosen from the menu -- this may be difficult/awkward given current dependence on cldrLoad.reloadV()
  • Further refactoring/clean-up; setMenuFromData() is over 100 lines long
  • Ideally modernize the client-server code, use API, avoid deprecated cldrLoad.myLoad()

Comment on lines +106 to 110
appendDiv(el, INFO_VOTE_TICKET_ID);
appendDiv(el, INFO_REGIONAL_ID);
appendDiv(el, INFO_FORUM_ID);
appendDiv(el, INFO_XPATH_ID);
}
Copy link
Member

Choose a reason for hiding this comment

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

ideally we should try to start moving these into InfoPanel.vue

Copy link
Member

Choose a reason for hiding this comment

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

but to add, good to add this subcomponent into a vue component

@btangmu btangmu merged commit 3dc8edd into unicode-org:main Oct 31, 2023
8 checks passed
@btangmu btangmu deleted the t17080_bb branch October 31, 2023 13:29
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