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.

Sign up for GitHub

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

Jump to bottom

feat: Adapt to mobile, backspace to hide the menu, generally mobile does not have esc key. #987

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

feat: Adapt to mobile, backspace to hide the menu, generally mobile does not have esc key. #987

wants to merge 4 commits into from

Conversation

jimmy54
Copy link

@jimmy54 jimmy54 commented Feb 26, 2025

Pull request

Issue tracker

Fixes will automatically close the related issue

Fixes #

Feature

Describe feature of pull request

Unit test

  • Done

Manual test

  • Done

Code Review

  1. Unit and manual test pass
  2. GitHub Action CI pass
  3. At least one contributor reviews and votes
  4. Can be merged clean without conflicts
  5. PR will be merged by rebase upstream base

Additional Info

eagleoflqj
eagleoflqj requested changes Feb 28, 2025
View reviewed changes
src/rime_api.h Outdated
@@ -168,6 +168,7 @@ typedef struct RIME_FLAVORED(rime_status_t) {
char* schema_name;
Bool is_disabled;
Bool is_composing;
Bool is_predicting;
Copy link
Member

Choose a reason for hiding this comment

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

This has to be added at the end, otherwise breaks ABI.

Copy link
Author

Choose a reason for hiding this comment

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

好的，我修改一下。

src/rime/switcher.cc Outdated
@@ -72,7 +72,7 @@ ProcessResult Switcher::ProcessKeyEvent(const KeyEvent& key_event) {
int ch = key_event.keycode();
if (ch == XK_space || ch == XK_Return) {
context_->ConfirmCurrentSelection();
} else if (ch == XK_Escape) {
} else if (ch == XK_Escape || ch == XK_BackSpace) {
Copy link
Member

Choose a reason for hiding this comment

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

Seems not reasonable. Your frontend should have the ability to hide itself.

Copy link
Author

Choose a reason for hiding this comment

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

前端确实有功能隐藏，就是比较麻烦。得先判断switcher已经显示，再手动发送XK_Escape按键。在switcher.cc直接添加，我觉是得更好的办法。一，不破坏原本逻辑。二，对所有移动端友好。

Copy link
Member

Choose a reason for hiding this comment

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

我觉得没道理。

退格键是删除的意思但是这里没有输入。
他跟取消的含义不同，有时候能达到同样的结果那是因为正好删掉了当前状态显示的内容。

编辑之外这两个键也可能代表不同操作，比如用退格键回退到流程的上一层界面，ESC键可能是取消整个流程。

switcher 我设想中要允许输入命令。退格键现在没用，可留待今后扩展。现在弄混，就被动了。

Copy link
Author

Choose a reason for hiding this comment

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

嗯，我理解的输入是用户要激活switcher需要向rime发送一个hotkey，然后Rime的向用户返回了结果，且结果是以candidate展现的，同时preedit也被修改了。这时再输入退格键，其实是用户取消了。
假如后续添加多层界面，用退格键做返回感觉也不冲突。

现在遇到问题是rime现在没有一个状态显示switcher已经激活。所以一般移动端的做法是。先读取一下hotkey-》监控用户是否输入hotkey-》switcher激活-》如果要取消的话就向rime发送esc键。上面的逻辑在没有配置esc的移动端都需要写一遍。正常移动端来讲都不会配置esc键。

Copy link
Member

Choose a reason for hiding this comment

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

现在遇到问题是rime现在没有一个状态显示switcher已经激活。

这个确实有必要。粗看了一下实现，貌似不太容易加。

所以一般移动端的做法是。先读取一下hotkey-》监控用户是否输入hotkey-》switcher激活-》如果要取消的话就向rime发送esc键。上面的逻辑在没有配置esc的移动端都需要写一遍。正常移动端来讲都不会配置esc键。

没做过，不理解。
如果说前端能打开方案选单，那么关闭比打开还要简单吧。
还是说，用户得自己按快捷键打开方案选单？用户能按出打开方案选单的键，但是按不出 esc？
键盘是自己开发的吗，能加上 esc 键吗？

Copy link
Author

Choose a reason for hiding this comment

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

是的，用户一般是按快捷键来调出方案选单的。一般来说移动端的虚拟键盘都不会配置esc键的，因为esc键用处不大，还占地方。
如果能加一个状态标识那更好了。用户能根据当前状态做选择，且在虚拟键盘上有UI上的变化。比如当前是激活状态，那么虚拟键盘上的退格键就显示取消菜单。用户点击取消菜单按钮会向rime发送esc的键值。

@eagleoflqj eagleoflqj requested a review from lotem February 28, 2025 04:32
jimmy54
jimmy54 commented Mar 5, 2025
View reviewed changes
Copy link
Author

@jimmy54 jimmy54 left a comment

Choose a reason for hiding this comment

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

It has been revised

@jimmy54 jimmy54 requested a review from eagleoflqj March 5, 2025 05:56
@fxliang
Copy link
Contributor

fxliang commented Mar 11, 2025

感觉上，这个更多应该在前端处理更简单，不然容易潜在影响其他前端出现问题。

比如前面在当前是在方案选单状态下就退格转换为escape来处理这样即可

@jimmy54
Copy link
Author

jimmy54 commented Mar 12, 2025

方案选单状态

上面已经讨论了。现在没有方案选单状态的相关api。

@fxliang
Copy link
Contributor

fxliang commented Mar 12, 2025

status.schema_id == ".default"

image

@jimmy54
Copy link
Author

jimmy54 commented Mar 13, 2025

status.schema_id == ".default"

image

不是方案，判断当前的候选是否是switcher。

@fxliang
Copy link
Contributor

fxliang commented Mar 13, 2025 •
edited
Loading 

./src/rime/switcher.cc
24:  context_->set_option("dumb", true);  // not going to commit anything

./src/rime/context.cc
29:  if (get_option("dumb"))

我认为 status.schema_id == ".default" 加上 rime_api->get_option(session_id, "dumb") 足够判断了

硬编码一个backspace进去，总体看上去有点奇奇怪怪的~

@jimmy54
Copy link
Author

jimmy54 commented Mar 13, 2025 

./src/rime/switcher.cc
24:  context_->set_option("dumb", true);  // not going to commit anything

./src/rime/context.cc
29:  if (get_option("dumb"))

我认为 status.schema_id == ".default" 加上 rime_api->get_option(session_id, "dumb") 足够判断了

硬编码一个backspace进去，总体看上去有点奇奇怪怪的~

嗯，感谢上面代码，有空我试试。如果上面判断准确的话，是否可以按照上面的条件添加一个is_switching在RimeStatus呢。另外，我个人觉得switcher添加backspace挺合理的阿。因为switcher本来是以候选的形式展示在候选栏的，backspace相当于删除操作。和正常的输入流程是一样的。

@lotem
Copy link
Member

lotem commented Mar 13, 2025

最近比較狼狽啊，囤的鍵盤需要挪挪，沒時間看。
先回覆樓上。

首先要承認，接口不完備，最好要有探測和開關方案選單的能力。
可是暫時沒有，就只能用技巧。

至於，退格鍵是刪除的動作，ESC 是是退出的動作，含義明確、各司其職，是最好的。
一般地，方案選單是用快捷鍵打開，而不是個字符鍵，那麼關閉選單用刪除就牽強了。

以將來的可能而論，要允許在方案選單裏輸入，搜索方案或者執行命令，倘若刪除鍵兼任退出的動作，連續刪除輸入碼就容易退出方案選單；然而只用 ESC 退出，信號就非常明確，按退格鍵刪除也可以放心大膽。

@jimmy54
Copy link
Author

jimmy54 commented Mar 17, 2025

作者以上所说流程很合理。如果只有一个输入码，用退格键隐藏也是合理的。不过我的代码没有体现出来。我先回退退格键的代码。另外的改动是否可以合并呢？

@jimmy54
Copy link
Author

jimmy54 commented Mar 18, 2025

switcher代码回退。请作者看看另外的pr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lotem lotem Awaiting requested review from lotem

@eagleoflqj eagleoflqj Awaiting requested review from eagleoflqj

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jimmy54 @fxliang @lotem @eagleoflqj @jimmy8854