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

Fixed layouts indexes in Types.kt #222

Merged
merged 1 commit into from
May 13, 2023

Conversation

Chirikumbrah
Copy link
Contributor

Now all layouts should be displayed correctly :)

Copy link
Contributor

@roihershberg roihershberg left a comment

Choose a reason for hiding this comment

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

Hi! While working on a Hebrew layout I noticed that the only change you need to make in this file is changing the index of the ThumbKeyFIv1Wide layout to 22. This is due to this commit which adds the layout but not assigns a new index: 2e76ba4

@Chirikumbrah
Copy link
Contributor Author

@roihershberg, the problem was in the 18th index (suomi wide & ru with symbols were the same), so I added +1 to every layout created by me)

@Chirikumbrah
Copy link
Contributor Author

@roihershberg, yeah, now I see. I've looked at the changes and u're right)
I'll leave it as is for now, but anyway, thank you for your suggestion)

@roihershberg
Copy link
Contributor

I think that the problem when messing with the indexes is that it will change the chosen layouts in the settings. I haven't looked at the code but I'm almost certain that the chosen layouts are stored in the shared preferences by their index. That's how I discovered this bug.
So every layout that the index have been changed for in this PR and a user selected them as the layouts he wants to use, will now be a different layout and the user may suddenly see a completely different language and will not understand what happened.
Therefore, a change in the indexes should be as minimal as possible.

Try a version of this app before your changes, choose the layouts that you've changed the index for and then install the version of the app with your changes and see if you now have different layouts or a different order of the layouts.

@Chirikumbrah
Copy link
Contributor Author

Try a version of this app before your changes, choose the layouts that you've changed the index for and then install the version of the app with your changes and see if you now have different layouts or a different order of the layouts.

I did that)
As I said before the problem was here)
Screenshot-2023-05-13-17-23-05.jpeg

Now it's ok and I'm using version built with files from this PR)

@dessalines dessalines merged commit bc24020 into dessalines:main May 13, 2023
@roihershberg
Copy link
Contributor

@Chirikumbrah @dessalines
Yeah this is the problem but only ThumbKeyFIv1Wide should be changed to 22.
This should be reverted. The problem that I was suspecting is correct. The chosen layouts of the users will be changed.
Look at the video demonstrating how the layouts are changing after the patch:

patch-effect-on-layouts.mp4

@dessalines
Copy link
Owner

Could you do a PR to fix this?

@roihershberg
Copy link
Contributor

Yes

@dessalines
Copy link
Owner

dessalines commented May 13, 2023

No layouts should have changed indices as of the 1.0.0 release.

mpice-mn pushed a commit to mpice-mn/thumb-key that referenced this pull request Sep 23, 2023
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.

3 participants