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

Add Landscape Keyboard #1191

Merged
merged 15 commits into from
Jan 23, 2024
Merged

Add Landscape Keyboard #1191

merged 15 commits into from
Jan 23, 2024

Conversation

goofyz
Copy link
Collaborator

@goofyz goofyz commented Jan 22, 2024

Pull request

Issue tracker

Fixes will automatically close the related issues

Fixes #1038
Fixes #1111
Refs #1148

Feature

  • Add "Landscape Keyboard" related options in "Setting"
  • Allow manually appoint "Landscape Keyboard" in Theme file (`trime.custom.yaml")
  • Allow auto-split keyboard

Code of conduct

Style lint

  • make sytle-lint

Build pass

  • make debug

Manually test

  • Done

Code Review

  1. No wildcards import
  2. Manual build and test pass
  3. GitHub action ci pass
  4. At least one contributor reviews and votes
  5. Can be merged clean without conflicts
  6. PR will be merged by rebase upstream base

Daily build

Login and download artifact at https://github.com/osfans/trime/actions

Additional Info

@cabins @isPoto If possible, please help to test and comment. Thanks.

@shitlime
Copy link
Contributor

LGTM

@WhiredPlanck
Copy link
Collaborator

WhiredPlanck commented Jan 22, 2024

I really appreciate your work so much, but I am worrying about the UI components which are getting more massive and unmaintainable. Refactoring the whole input view to improve the performance, etc may be the more urged things we need to do first (I am doing so).

@goofyz
Copy link
Collaborator Author

goofyz commented Jan 22, 2024

I really appreciate your work so much, but I am worrying about the UI components which are getting more massive and unmaintainable. Refactoring the whole input view to improve the performance, etc may be the more urged things we need to do first (I am doing so).

agree.
i try to decouple some logic from Keyboard too. And this PR do not add much to the logic (i think)

Copy link
Collaborator

@Bambooin Bambooin left a comment

Choose a reason for hiding this comment

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

No major issues found except #1188 which was already fixed.

I don't verify the whole function, so please do the manually test carefully

@WhiredPlanck WhiredPlanck merged commit ca8959e into osfans:develop Jan 23, 2024
3 checks passed
@Bambooin
Copy link
Collaborator

We'd better keep the commit history, except for the messy commits we can squash it.

The commit message of this pull request are very clear and well organized.

@WhiredPlanck
Copy link
Collaborator

We'd better keep the commit history, except for the messy commits we can squash it.

The commit message of this pull request are very clear and well organized.

You are right. But just because rebase will break the integrity of the commits, I wanted to try the squashing.

@nopdan
Copy link
Contributor

nopdan commented Jan 23, 2024

此项提交导致空键占位高度异常。
{width: 100, height: 1}
height 值无效(貌似是回退到了 key_height)

@WhiredPlanck
Copy link
Collaborator

此项提交导致空键占位高度异常。 {width: 100, height: 1} height 值无效(貌似是回退到了 key_height)

没事,能修就行。不怕出问题,就怕修不好。

@goofyz
Copy link
Collaborator Author

goofyz commented Jan 24, 2024

此项提交导致空键占位高度异常。 {width: 100, height: 1} height 值无效(貌似是回退到了 key_height)

我試了 {width: 100, height: 1} 沒有問題。
留意必須為新一行 height 才有作用。

@nopdan
Copy link
Contributor

nopdan commented Jan 24, 2024

Screenrecorder-2024-01-24-15-35-21-524.mp4

试了一下用默认主题,一切正常,我再找找复现条件

@nopdan
Copy link
Contributor

nopdan commented Jan 24, 2024

此项提交导致空键占位高度异常。 {width: 100, height: 1} height 值无效(貌似是回退到了 key_height)

我試了 {width: 100, height: 1} 沒有問題。 留意必須為新一行 height 才有作用。

你试试 {width: 100, height: 0}

@goofyz goofyz deleted the fix-1038 branch January 24, 2024 08:41
@goofyz
Copy link
Collaborator Author

goofyz commented Jan 24, 2024

明白,打到原因了,遲點 submit PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants