Skip to content

apply theme to TextEditingUi + editable clipboard content radius #489

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

Merged
merged 3 commits into from
Jul 5, 2024

Conversation

ccyybn
Copy link
Contributor

@ccyybn ccyybn commented Apr 28, 2024

Text Editing UI

image
image
image
image

Symbol Key Border

image
image
image

Clipboard Content Radius

image

Copy link
Contributor

@mokapsing mokapsing left a comment

Choose a reason for hiding this comment

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

app/src/main/java/org/fcitx/fcitx5/android/input/keyboard/KeyDef.kt 64行,radius未传值,93行同理

@ccyybn
Copy link
Contributor Author

ccyybn commented Apr 28, 2024

app/src/main/java/org/fcitx/fcitx5/android/input/keyboard/KeyDef.kt 64行,radius未传值,93行同理

加了

@ccyybn
Copy link
Contributor Author

ccyybn commented Apr 28, 2024

禁用按键逻辑 与 Gboard 一致

show.mp4

@mokapsing
Copy link
Contributor

禁用按键逻辑 与 Gboard 一致

show.mp4

希望不要影响类似termux这种应用中的表现,例如上下切换history等

@rocka
Copy link
Member

rocka commented Apr 29, 2024

这个 Select 键还有些问题,原本的实现是,只有在编辑器中有选择的文字时,才会变成蓝色(accentKeyBackgroundColor),取消选择后,就会变回普通按键的颜色

@ccyybn
Copy link
Contributor Author

ccyybn commented Apr 29, 2024

这个 Select 键还有些问题,原本的实现是,只有在编辑器中有选择的文字时,才会变成蓝色(accentKeyBackgroundColor),取消选择后,就会变回普通按键的颜色

这个改好了

@rocka
Copy link
Member

rocka commented May 2, 2024

没开启按键边框的时候,原来按钮之间的分割线都没了:

另外,这里的按钮边距应该需要统一一下,感觉没必要跟随按键横向/纵向边距的设置,或者这个界面就不该复用 KeyView ...

而且感觉按钮的字体没必要用粗体 ...

@ccyybn
Copy link
Contributor Author

ccyybn commented May 2, 2024

没开启按键边框的时候,原来按钮之间的分割线都没了:

另外,这里的按钮边距应该需要统一一下,感觉没必要跟随按键横向/纵向边距的设置,或者这个界面就不该复用 KeyView ... 而且感觉按钮的字体没必要用粗体 ...

审美差异吧,我觉得粗体好看一点,好像主键盘的符号切换键也是粗体,那个按钮还更小一点

没分割线是因为风格与主题统一,可能接受的人会多一些吧

纵向边距我主键盘也调小了的,看起来没那没那么维和,不过确实单独调整更好,横纵一样宽,这个我会加个选项控制

不复用KeyView改动有点大,如果要这样改并同时满足之前分割线的效果,我觉得很难达到代码要求,可能你来提交要方便一些

@ccyybn ccyybn force-pushed the fcitx5_pr branch 3 times, most recently from d8a3dd7 to ad31556 Compare May 3, 2024 18:29
@ccyybn
Copy link
Contributor Author

ccyybn commented May 3, 2024

@rocka 现在我加了选项,3种按钮随便选,每个人应该都满意了,你可以试试

image

@ccyybn ccyybn force-pushed the fcitx5_pr branch 3 times, most recently from 365a08f to e3d773f Compare May 3, 2024 19:37
@ccyybn
Copy link
Contributor Author

ccyybn commented May 4, 2024

@rocka

我下载 Release版 0.0.9 确认了下,之前线条效果的文本编辑按键有个bug,就是在我的平板上,这些线条是不可见的

image

@ccyybn ccyybn force-pushed the fcitx5_pr branch 3 times, most recently from 223493f to ecfc97e Compare May 4, 2024 14:33
@rocka
Copy link
Member

rocka commented Jul 5, 2024

首先我得先道个歉,最近确实比较忙,没时间处理 PR ,尤其是这种规模比较大的 ... 然后就要开始挑刺儿了。


虽然之前提交的代码“看起来”达成了描述中的效果,但落到实现细节上,有些地方对已有封装类的使用有误,破坏了整体的抽象层次。比如,在 TextEditingUi

class TextEditingUi(override val ctx: Context, private val theme: Theme) : Ui {
//if the main keyboard border is off and text editing is the border key, the background color should be applied as barColor
private val keyBorder by ThemeManager.prefs.keyBorder
//keyBordered should always be true, because the rounded rectangle pressed effect is needed even if borderless is selected
private val keyBordered =
ManagedPreference.PBool(PreferenceManager.getDefaultSharedPreferences(ctx), "", true)

KeyDef

sealed class Appearance(
val percentWidth: Float,
val variant: Variant,
val border: Border,
val margin: Boolean,
val prefs: Prefs,
val viewId: Int,
val soundEffect: InputFeedbacks.SoundEffect
) {
class Prefs(
val keyBorder: ManagedPreference.PBool = ThemeManager.prefs.keyBorder,
val keyRippleEffect: ManagedPreference.PBool = ThemeManager.prefs.keyRippleEffect,
val keyRadius: ManagedPreference.PInt = ThemeManager.prefs.keyRadius,
val keyHorizontalMargin: ManagedPreference.PInt = ThemeManager.prefs.keyHorizontalMargin,
val keyVerticalMargin: ManagedPreference.PInt = ThemeManager.prefs.keyVerticalMargin,
val keyHorizontalMarginLandscape: ManagedPreference.PInt = ThemeManager.prefs.keyHorizontalMarginLandscape,
val keyVerticalMarginLandscape: ManagedPreference.PInt = ThemeManager.prefs.keyVerticalMarginLandscape,
)

中使用 ManagedPreferenceManagedPreference 是对 SharedPreferences 的封装,为其供类型以及选项值改变的监听,只应该在定义应用设置项时,比如 AppPrefsThemePrefs 中使用,不应该被下放到 UI 的实现中。而 KeyDef 承担了定义键盘布局中按键外观与功能的责任,如非必要,不应该在其中定义 UI 中使用的具体数字(比如圆角半径的 px 或 dp 值),更不应该出现 Android 中的类对象(比如 SharedPreferences)。

其次,对 Splitties View DSLUi 接口实现,或者更笼统地说,可复用的 UI 组件的实现,应该只关心在界面上显示的内容,不应该与其它业务逻辑耦合。例如 PickerPageUi

class PickerPageUi(
override val ctx: Context,
val theme: Theme,
private val density: Density,
recentlyUsedFileName: String = ""
) : Ui {
private val keyBorder by ThemeManager.prefs.keyBorder
private val keySymbolBorder =
ThemeManager.prefs.keySymbolBorder.getValue() && recentlyUsedFileName == PickerWindow.Key.Symbol.name

中就不应该将 recentlyUsedFileName 作为构造函数的参数传入。如果你想根据 recentlyUsedFileName 的值来控制边框,在外层传入一个 Boolean 类型足矣。

另外,没必要每种实现都加个选项,让符号键盘和文本编辑界面都跟随键盘的按键边框设置就够了,也没必要每个边距都独立成一个设置,否则设置界面的膨胀速度就不可控了。

最后,每个 PR 应该只包含一个功能的实现,不应该把多个互不相关的功能放在一起,否则会给 review 增加不必要的工作量(要额外区分哪些文件实现了哪些功能),也不方便合并的时候直接 squash 。


代码我已经全都重构成比较符合目前编码风格的实现了,如果有问题可以再商量,没问题的话就按照现在的实现合并了(可能还需要再 rebase 一下,把相关的功能 squash 成一个 commit)。

@ccyybn
Copy link
Contributor Author

ccyybn commented Jul 5, 2024

@rocka 一切按照你改的来,这代码我改起来头疼,最近也比较忙

rocka and others added 2 commits July 6, 2024 03:02
Co-authored-by: cc <ccyybn@gmail.com>
Co-authored-by: cc <ccyybn@gmail.com>
Co-authored-by: cc <ccyybn@gmail.com>
@rocka rocka merged commit 57a15d8 into fcitx5-android:master Jul 5, 2024
17 checks passed
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