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

Allow loading language files with two part language code #339

Merged
merged 3 commits into from
Apr 22, 2022

Conversation

TPiUnikie
Copy link
Contributor

@TPiUnikie TPiUnikie commented Apr 19, 2022

Change the imported filenames to inlcude underscore in two part language names. #871

Signed-off-by: Timo Piiroinen [email protected]


Here's what your changelog entry will look like:

🐛 Bug Fixes

  • Allow loading language files with two part language code (#339). Contributed by @TPiUnikie.

…age names. (issue #19218)

Signed-off-by: Timo Piiroinen <[email protected]>
@TPiUnikie TPiUnikie requested a review from a team as a code owner April 19, 2022 07:38
@dbkr dbkr changed the title Allows loading language files with to part language code Allows loading language files with two part language code Apr 19, 2022
}
return part.join("_");
}
return locale;
Copy link
Member

Choose a reason for hiding this comment

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

A few things about this function:

  • The if branch is redundant? The code only does the split if the string contains a hyphen, so we know we'll get at least two parts.
  • Having langDesc & partsReq as defined constants I think might be adding more confusion. I'd probably just use the numbers straight in the code (if partsReq is even necessary).
  • Maybe parts instead of part for the variable - it's all the parts of the split string
  • Could it get a comment saying what it does?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • The first if was like a guard to avoid variables to be reserved if there is no hyphen, maybe those will be optimized out?
  • maybe langDesc rename to localeIndex. Currently the second part is for uppercase.
    If there are cases like aa_bb_cc, then it would output aa-BB-cc, using last index: aa-bb-CC.
  • Parts would be a better name
  • partsReq can be removed, if change the > langDesc is changed to comparison

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, although I'm not sure the if is really optimising anything: variables are very cheap, so this will probably spend more time searching through the string twice. In any case, it would be a tiny optimisation: code legibility is much more important. I don't really mind though, although if you do want to do it then we should do it with the more modern includes() rather than indexOf().

Also I'm afraid I still don't understand the need for the localeSubIndex const: any actual constants should be near the top of the file above the class, but for the second thing in an array you can just say parts[1] which is clearer and more concise.

A comment on the function saying what it does would still be very useful ('denormalize' doesn't really tell me what the function is supposed to do).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added comment
Removed the 'if' check
Used fixed index numbers instead of defined constant

Signed-off-by: Timo Piiroinen <[email protected]>
@t3chguy t3chguy requested a review from dbkr April 20, 2022 11:34
@dbkr dbkr changed the title Allows loading language files with two part language code Allow loading language files with two part language code Apr 21, 2022
@dbkr
Copy link
Member

dbkr commented Apr 21, 2022

I've also changed the tense of the PR title to match the standard for our changelogs.

Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

Thanks for making those changes, to me this is a lot easier to understand.

@dbkr dbkr added the T-Defect label Apr 22, 2022
@dbkr dbkr merged commit fcfda67 into element-hq:develop Apr 22, 2022
su-ex added a commit to SchildiChat/element-desktop that referenced this pull request May 10, 2022
* Made the location map change the cursor to a pointer so it looks like it's clickable (https ([\#8451](matrix-org/matrix-react-sdk#8451)). Fixes element-hq/element-web#21991. Contributed by @Odyssey346.
* Implement improved spacing for the thread list and timeline ([\#8337](matrix-org/matrix-react-sdk#8337)). Fixes element-hq/element-web#21759. Contributed by @luixxiul.
* LLS: expose way to enable live sharing labs flag from location dialog ([\#8416](matrix-org/matrix-react-sdk#8416)).
* Fix source text boxes in View Source modal should have full width ([\#8425](matrix-org/matrix-react-sdk#8425)). Fixes element-hq/element-web#21938. Contributed by @EECvision.
* Read Receipts: never show +1, if it’s just 4, show all of them ([\#8428](matrix-org/matrix-react-sdk#8428)). Fixes element-hq/element-web#21935.
* Add opt-in analytics to onboarding tasks ([\#8409](matrix-org/matrix-react-sdk#8409)). Fixes element-hq/element-web#21705.
* Allow user to control if they are signed out of all devices when changing password ([\#8259](matrix-org/matrix-react-sdk#8259)). Fixes element-hq/element-web#2671.
* Implement new Read Receipt design ([\#8389](matrix-org/matrix-react-sdk#8389)). Fixes element-hq/element-web#20574.
* Stick connected video rooms to the top of the room list ([\#8353](matrix-org/matrix-react-sdk#8353)).
* LLS: fix jumpy maximised map ([\#8387](matrix-org/matrix-react-sdk#8387)).
* Persist audio and video mute state in video rooms ([\#8376](matrix-org/matrix-react-sdk#8376)).
* Forcefully disconnect from video rooms on logout and tab close ([\#8375](matrix-org/matrix-react-sdk#8375)).
* Add local echo of connected devices in video rooms ([\#8368](matrix-org/matrix-react-sdk#8368)).
* Improve text of account deactivation dialog ([\#8371](matrix-org/matrix-react-sdk#8371)). Fixes element-hq/element-web#17421.
* Live location sharing: own live beacon status on maximised view ([\#8374](matrix-org/matrix-react-sdk#8374)).
* Show a lobby screen in video rooms ([\#8287](matrix-org/matrix-react-sdk#8287)).
* Settings toggle to disable Composer Markdown ([\#8358](matrix-org/matrix-react-sdk#8358)). Fixes element-hq/element-web#20321.
* Cache localStorage objects for SettingsStore ([\#8366](matrix-org/matrix-react-sdk#8366)).
* Bring `View Source` back from behind developer mode ([\#8369](matrix-org/matrix-react-sdk#8369)). Fixes element-hq/element-web#21771.
* Fix update from creating desktop shortcut ([\element-hq#333](element-hq#333)). Fixes element-hq/element-web#9210. Contributed by @elibroftw.
* Fix macOS and Linux build regressions ([\element-hq#345](element-hq#345)).
* Allow loading language files with two part language code ([\element-hq#339](element-hq#339)). Contributed by @TPiUnikie.
* Fix Jitsi Meet getting wedged at startup in some cases ([\#21995](element-hq/element-web#21995)).
* Fix camera getting muted when disconnecting from a video room ([\#21958](element-hq/element-web#21958)).
* Fix race conditions around threads ([\#8448](matrix-org/matrix-react-sdk#8448)). Fixes element-hq/element-web#21627.
* Fix reading of cached room device setting values ([\#8495](matrix-org/matrix-react-sdk#8495)).
* Fix issue with dispatch happening mid-dispatch due to js-sdk emit ([\#8473](matrix-org/matrix-react-sdk#8473)). Fixes element-hq/element-web#22019.
* Match MSC behaviour for threads when disabled (thread-aware mode) ([\#8476](matrix-org/matrix-react-sdk#8476)). Fixes element-hq/element-web#22033.
* Specify position of DisambiguatedProfile inside a thread on bubble message layout ([\#8452](matrix-org/matrix-react-sdk#8452)). Fixes element-hq/element-web#21998. Contributed by @luixxiul.
* Location sharing: do not trackuserlocation in location picker ([\#8466](matrix-org/matrix-react-sdk#8466)). Fixes element-hq/element-web#22013.
* fix text and map indent in thread view ([\#8462](matrix-org/matrix-react-sdk#8462)). Fixes element-hq/element-web#21997.
* Live location sharing: don't group beacon info with room creation summary ([\#8468](matrix-org/matrix-react-sdk#8468)).
* Don't linkify code blocks ([\#7859](matrix-org/matrix-react-sdk#7859)). Fixes element-hq/element-web#9613.
* read receipts: improve tooltips to show names of users ([\#8438](matrix-org/matrix-react-sdk#8438)). Fixes element-hq/element-web#21940.
* Fix poll overflowing a reply tile on bubble message layout ([\#8459](matrix-org/matrix-react-sdk#8459)). Fixes element-hq/element-web#22005. Contributed by @luixxiul.
* Fix text link buttons on UserInfo panel ([\#8247](matrix-org/matrix-react-sdk#8247)). Fixes element-hq/element-web#21702. Contributed by @luixxiul.
* Clear local storage settings handler cache on logout ([\#8454](matrix-org/matrix-react-sdk#8454)). Fixes element-hq/element-web#21994.
* Fix jump to bottom button being always displayed in non-overflowing timelines ([\#8460](matrix-org/matrix-react-sdk#8460)). Fixes element-hq/element-web#22003.
* fix timeline search with empty text box should do nothing ([\#8262](matrix-org/matrix-react-sdk#8262)). Fixes element-hq/element-web#21714. Contributed by @EECvision.
* Fixes "space panel kebab menu is rendered out of view on sub spaces"  ([\#8350](matrix-org/matrix-react-sdk#8350)). Contributed by @yaya-usman.
* Add margin to the location map inside ThreadView ([\#8442](matrix-org/matrix-react-sdk#8442)). Fixes element-hq/element-web#21982. Contributed by @luixxiul.
* Patch: "Reloading the registration page should warn about data loss" ([\#8377](matrix-org/matrix-react-sdk#8377)). Contributed by @yaya-usman.
* Live location sharing: fix safari timestamps pt 2 ([\#8443](matrix-org/matrix-react-sdk#8443)).
* Fix issue with thread notification state ignoring initial events ([\#8417](matrix-org/matrix-react-sdk#8417)). Fixes element-hq/element-web#21927.
* Fix event text overflow on bubble message layout ([\#8391](matrix-org/matrix-react-sdk#8391)). Fixes element-hq/element-web#21882. Contributed by @luixxiul.
* Disable the message action bar when hovering over the 1px border between threads on the list ([\#8429](matrix-org/matrix-react-sdk#8429)). Fixes element-hq/element-web#21955. Contributed by @luixxiul.
* correctly align read receipts to state events in bubble layout ([\#8419](matrix-org/matrix-react-sdk#8419)). Fixes element-hq/element-web#21899.
* Fix issue with underfilled timelines when barren of content ([\#8432](matrix-org/matrix-react-sdk#8432)). Fixes element-hq/element-web#21930.
* Fix baseline misalignment of thread panel summary by deduplication ([\#8413](matrix-org/matrix-react-sdk#8413)).
* Fix editing of non-html replies ([\#8418](matrix-org/matrix-react-sdk#8418)). Fixes element-hq/element-web#21928.
* Read Receipts "Fall from the Sky" ([\#8414](matrix-org/matrix-react-sdk#8414)). Fixes element-hq/element-web#21888.
* Make read receipts handle nullable roomMembers correctly ([\#8410](matrix-org/matrix-react-sdk#8410)). Fixes element-hq/element-web#21896.
* Don't form continuations on either side of a thread root ([\#8408](matrix-org/matrix-react-sdk#8408)). Fixes element-hq/element-web#20908.
* Fix centering issue with sticker placeholder ([\#8404](matrix-org/matrix-react-sdk#8404)). Fixes element-hq/element-web#18014 and element-hq/element-web#6449.
* Disable download option on <video/> , preferring dedicated download button ([\#8403](matrix-org/matrix-react-sdk#8403)). Fixes element-hq/element-web#21902.
* Fix infinite loop when pinning/unpinning persistent widgets ([\#8396](matrix-org/matrix-react-sdk#8396)). Fixes element-hq/element-web#21864.
* Tweak ReadReceiptGroup to better handle disambiguation ([\#8402](matrix-org/matrix-react-sdk#8402)). Fixes element-hq/element-web#21897.
* stop the bottom edge of buttons getting clipped in devtools ([\#8400](matrix-org/matrix-react-sdk#8400)).
* Fix issue with threads timelines with few events cropping events ([\#8392](matrix-org/matrix-react-sdk#8392)). Fixes element-hq/element-web#20594.
* Changed font-weight to 400 to support light weight font ([\#8345](matrix-org/matrix-react-sdk#8345)). Fixes element-hq/element-web#21171. Contributed by @goelesha.
* Fix issue with thread panel not updating when it loads on first render ([\#8382](matrix-org/matrix-react-sdk#8382)). Fixes element-hq/element-web#21737.
* fix: "Mention highlight and cursor hover highlight has different corner radius" ([\#8384](matrix-org/matrix-react-sdk#8384)). Contributed by @yaya-usman.
* Fix regression around haveRendererForEvent for hidden events ([\#8379](matrix-org/matrix-react-sdk#8379)). Fixes element-hq/element-web#21862 and element-hq/element-web#21725.
* Fix regression around the room list treeview keyboard a11y ([\#8385](matrix-org/matrix-react-sdk#8385)). Fixes element-hq/element-web#21436.
* Remove float property to let the margin between events appear on bubble message layout ([\#8373](matrix-org/matrix-react-sdk#8373)). Fixes element-hq/element-web#21861. Contributed by @luixxiul.
* Fix race in Registration between server change and flows fetch ([\#8359](matrix-org/matrix-react-sdk#8359)). Fixes element-hq/element-web#21800.
* fix rainbow breaks compound emojis ([\#8245](matrix-org/matrix-react-sdk#8245)). Fixes element-hq/element-web#21371. Contributed by @EECvision.
* Fix RightPanelStore handling first room on app launch wrong ([\#8370](matrix-org/matrix-react-sdk#8370)). Fixes element-hq/element-web#21741.
* Fix UnknownBody error message unalignment ([\#8346](matrix-org/matrix-react-sdk#8346)). Fixes element-hq/element-web#21828. Contributed by @luixxiul.
* Use -webkit-line-clamp for the room header topic overflow ([\#8367](matrix-org/matrix-react-sdk#8367)). Fixes element-hq/element-web#21852. Contributed by @luixxiul.
* Fix issue with ServerInfo crashing the modal ([\#8364](matrix-org/matrix-react-sdk#8364)).
* Fixes around threads beta in degraded mode ([\#8319](matrix-org/matrix-react-sdk#8319)). Fixes element-hq/element-web#21762.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants