Plume
/
Plume
8
16
Fork 22
Code Issues 174 Pull Requests 8 Releases 8 Wiki Activity

Labels Milestones

Fix issue #642, frontend not in english if the user language does not exist #648

Merged
requiem958 merged 2 commits from issue-frontend-language into master 5 years ago
Conversation 2 Commits 2 Files Changed 1
requiem958 commented 5 years ago (Migrated from github.com)

Fix #642

Iterate over catalogs to find the english index and then use it instead of 0.

Fix [#642](https://github.com/Plume-org/Plume/issues/642) Iterate over catalogs to find the english index and then use it instead of 0.
👍 1 ❤️ 1
codecov[bot] commented 5 years ago (Migrated from github.com)

Codecov Report

Merging #648 into master will increase coverage by 0.23%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #648      +/-   ##
==========================================
+ Coverage   35.11%   35.35%   +0.23%     
==========================================
  Files          68       68              
  Lines        7943     7915      -28     
  Branches     1889     1894       +5     
==========================================
+ Hits         2789     2798       +9     
+ Misses       4374     4341      -33     
+ Partials      780      776       -4
# [Codecov](https://codecov.io/gh/Plume-org/Plume/pull/648?src=pr&el=h1) Report > Merging [#648](https://codecov.io/gh/Plume-org/Plume/pull/648?src=pr&el=desc) into [master](https://codecov.io/gh/Plume-org/Plume/commit/28fb50438e53019d5bda97e3b2532e5831096c9e?src=pr&el=desc) will **increase** coverage by `0.23%`. > The diff coverage is `n/a`. ```diff @@ Coverage Diff @@ ## master #648 +/- ## ========================================== + Coverage 35.11% 35.35% +0.23% ========================================== Files 68 68 Lines 7943 7915 -28 Branches 1889 1894 +5 ========================================== + Hits 2789 2798 +9 + Misses 4374 4341 -33 + Partials 780 776 -4 ```
codecov[bot] commented 5 years ago (Migrated from github.com)

Codecov Report

Merging #648 into master will increase coverage by 0.23%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #648      +/-   ##
==========================================
+ Coverage   35.11%   35.35%   +0.23%     
==========================================
  Files          68       68              
  Lines        7943     7915      -28     
  Branches     1889     1894       +5     
==========================================
+ Hits         2789     2798       +9     
+ Misses       4374     4341      -33     
+ Partials      780      776       -4
# [Codecov](https://codecov.io/gh/Plume-org/Plume/pull/648?src=pr&el=h1) Report > Merging [#648](https://codecov.io/gh/Plume-org/Plume/pull/648?src=pr&el=desc) into [master](https://codecov.io/gh/Plume-org/Plume/commit/28fb50438e53019d5bda97e3b2532e5831096c9e?src=pr&el=desc) will **increase** coverage by `0.23%`. > The diff coverage is `n/a`. ```diff @@ Coverage Diff @@ ## master #648 +/- ## ========================================== + Coverage 35.11% 35.35% +0.23% ========================================== Files 68 68 Lines 7943 7915 -28 Branches 1889 1894 +5 ========================================== + Hits 2789 2798 +9 + Misses 4374 4341 -33 + Partials 780 776 -4 ```
igalic (Migrated from github.com) approved these changes 5 years ago
igalic (Migrated from github.com) left a comment

please ignore my rambling

please ignore my rambling
plume-front/src/main.rs
@ -47,2 +47,4 @@
let lang = js! { return navigator.language }.into_string().unwrap();
let lang = lang.splitn(2, '-').next().unwrap_or("en");
let english_position = catalogs
igalic (Migrated from github.com) commented 5 years ago

this should be either a (cached?) constant, or a (cached?) config value

this should be either a (cached?) constant, or a (cached?) config value
plume-front/src/main.rs
@ -49,0 +50,4 @@
let english_position = catalogs
.iter()
.position(|(language_code, _)| *language_code == "en")
.unwrap();
igalic (Migrated from github.com) commented 5 years ago

while i understand that this is a simple operation, i don't think we need to do it every time

while i understand that this is a simple operation, i don't think we need to do it every time
igalic (Migrated from github.com) commented 5 years ago

actually, never mind, this is an operation that's only dealing with things known at compile time

the optimiser will probably inline this as a constant literal

actually, never mind, this is an operation that's only dealing with things known at compile time the optimiser will probably inline this as a constant literal
requiem958 (Migrated from github.com) reviewed 5 years ago
plume-front/src/main.rs
@ -47,2 +47,4 @@
let lang = js! { return navigator.language }.into_string().unwrap();
let lang = lang.splitn(2, '-').next().unwrap_or("en");
let english_position = catalogs
requiem958 (Migrated from github.com) commented 5 years ago

It should be a constant yes, but the rust doc of constant says :

Constants can be set only to a constant expression and not to the result of a function call or any other value that will be computed at runtime.

So i thought the keyword const was forbidden in this case.

It should be a constant yes, but the rust doc of constant says : > Constants can be set only to a constant expression and not to the result of a function call or any other value that will be computed at runtime. So i thought the keyword const was forbidden in this case.
trinity-1686a reviewed 5 years ago
plume-front/src/main.rs
@ -47,2 +47,4 @@
let lang = js! { return navigator.language }.into_string().unwrap();
let lang = lang.splitn(2, '-').next().unwrap_or("en");
let english_position = catalogs
trinity-1686a commented 5 years ago
Owner

All this is in a lazy_static! call, so the result will be evaluated only one time, the first time CATALOG is accessed. The only thing it's not is cached between two different page loading, but that's not necessary imo

All this is in a lazy_static! call, so the result will be evaluated only one time, the first time CATALOG is accessed. The only thing it's not is cached between two different page loading, but that's not necessary imo

Reviewers

igalic
The pull request has been merged as 935d331e97.
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b issue-frontend-language master
git pull origin issue-frontend-language

Step 2:

Merge the changes and update on Forgejo.
git checkout master
git merge --no-ff issue-frontend-language
git push origin master
Sign in to join this conversation.
Reviewers
Request review
No reviewers
igalic
Labels
Apply labels
Clear labels   
A: API

Related to the REST API   
A: Backend

Code running on the server   
A: Federation

Stuff related to Federation   
A: Front-End

Related to the front-end   
A: I18N

Translations, and related code   
A: Meta

More about project management or code than the project itself   
A: Security
   
Build

The building, or installation process of Plume   
C: Bug

Something isn't working   
C: Discussion

We need to talk   
C: Enhancement

New feature or request   
C: Feature

This is a new feature   
Compatibility

Compatibility with different browsers, readers and OS   
Dependency

Related to an external package that Plume uses   
Design

UI/UX related issues and PRs   
Documentation
   
Good first issue

Good for newcomers   
Help welcome

Extra attention is needed   
Mobile

Issues affecting only mobile UX   
Rendering

How elements're rendered out for the end user   
S: Blocked

Something else needs to be fixed first   
S: Duplicate

This issue or pull request already exists   
S: Incomplete

This PR is not complete yet   
S: Instance specific

Issues concern a limited number of instances   
S: Invalid

This doesn't seem right   
S: Needs Voting/Discussion

Need to be discussed by the community (on Loomio)   
S: Ready for review

This PR is ready to be reviewed   
Suggestion

Proposed ideas worth considering   
S: Voted on Loomio

This is issue has been created after a vote on Loomio   
S: Wontfix

This will not be worked on
No Label
A: API
A: Backend
A: Federation
A: Front-End
A: I18N
A: Meta
A: Security
Build
C: Bug
C: Discussion
C: Enhancement
C: Feature
Compatibility
Dependency
Design
Documentation
Good first issue
Help welcome
Mobile
Rendering
S: Blocked
S: Duplicate
S: Incomplete
S: Instance specific
S: Invalid
S: Needs Voting/Discussion
S: Ready for review
Suggestion
S: Voted on Loomio
S: Wontfix
Milestone
Set milestone
Clear milestone
No items
No Milestone
Assignees
Assign users
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: Plume/Plume#648
Write Preview
Loading…
Cancel
Save
There is no content yet.
Delete Branch 'issue-frontend-language'

Deleting a branch is permanent. It CANNOT be undone. Continue?

No
Yes