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

修复菜单左右空白不一致的草稿 #3

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lihaohong6
Copy link
Contributor

尝试修复go-musicfox/go-musicfox#223

我看了一下源代码,计算itemMaxLen的时候menuStartColumn没有*2,因此程序只考虑左侧的空白并假设右侧的空白为0,最终导致itemMaxLen过高,输出的时候第一列太长,留给第二列的空间不够(例如截图所示)。

image

加上*2之后并没有完全解决问题,比如那个我也不知道为什么好用的+6。理论上屏幕的宽度减去左右的空白(-menuStartColumn*2),再减去中间的4个空格(-4),得出的就是可以用来展示菜单的字符数,最后再把它除以二,平分给两列即可,但是实际上+6的效果更好。我怀疑是后面什么地方给前面的计算问题打了补丁,所以这里也必须把itemMaxLen算大一点。

当前版本还有不少问题,尤其是初始菜单:因为每一项都很短,所以整个菜单看起来偏左。一个解决方案是根据当页菜单的内容长短决定menuStartColumn是什么,但是会导致之前提过的菜单左右跳动的问题(尤其是翻页的时候)。还有一个方案是在菜单周围加个框,让用户能看到菜单的边界是什么。我不太确定怎么搞最好,所以先把草稿发出了看看反馈。

@anhoder
Copy link
Owner

anhoder commented Feb 7, 2025

引入的左右跳动问题感觉比原问题好像更加难以接受 (不过我也没想到其他的方案)

@anhoder
Copy link
Owner

anhoder commented Feb 7, 2025

原 issue 是单列显示靠右,是否可以只针对单列显示做下处理

@lihaohong6
Copy link
Contributor Author

我在 https://github.com/lihaohong6/go-musicfox 里重写了一下逻辑,用一个设置控制是否使用居中布局,效果如下。

image
image

在部分菜单中还是会出问题,例如,如果10首歌里只有一个歌名较长,布局会显得很奇怪(左边字多,右边字少)。考虑到当前的变化已经够大,我在想要不要先把它合并进去并且默认不启用,在ini配置文件里面注明这是个实验性的功能,未来可能会大改,然后再慢慢修这些问题。

还有就是spotifox已经被存档了,所以foxful-cli是否还有独立建repo的必要?当前的项目结构很不方便,我给foxful-cli的PR都必须先用musicfox测试,然后再复制粘贴过来。有些musicfox里的变化依赖foxful-cli,所以要走3步流程才能完成更改:

  1. 提交foxful-cli的更改。
  2. 让musicfox使用新版foxful-cli。
  3. 提交musicfox的更改。

如果保持现状的话,我就把我在 https://github.com/lihaohong6/go-musicfox 的4个commit拆成两个PR,一个提交给foxful-cli,另一个等musicfox更新foxful-cli之后再提交。

@anhoder
Copy link
Owner

anhoder commented Feb 23, 2025

  1. 可以先合并一版看看,默认不启用
  2. 本地测试可以在go.mod中用replace,ready后再同步到musicfox中

@lihaohong6
Copy link
Contributor Author

好的,我这周事情比较多,过几天更新下这个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
Development

Successfully merging this pull request may close these issues.

2 participants