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

Disable some menu items in read-only mode #11733

Merged
merged 20 commits into from
Jul 5, 2024

Conversation

Zuoqiu-Yingyi
Copy link
Contributor

  • Please commit to the dev branch
  • For contributing new features, please supplement and improve the corresponding user guide documents
  • For bug fixes, please describe the problem and solution via code comments
  • For text improvements (such as typos and wording adjustments), please submit directly

REL: #11367 #11707

在内核只读模式 & 发布模式下禁用会导致 Forbidden 提示的菜单项

@88250
Copy link
Member

88250 commented Jun 15, 2024

提交完了可以 review 的时候说一声,谢谢。

@Zuoqiu-Yingyi
Copy link
Contributor Author

提交完了可以 review 的时候说一声,谢谢。

可以 review 了

@88250 88250 added this to the 3.1.0 milestone Jun 16, 2024
Copy link
Member

@Vanessa219 Vanessa219 left a comment

Choose a reason for hiding this comment

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

请进行相关修改

fontFamilyHTML += `<option value="${item}"${window.siyuan.config.editor.fontFamily === item ? " selected" : ""}>${item}</option>`;
});
fontFamilyElement.innerHTML = fontFamilyHTML;
if (response.code === 0) {
Copy link
Member

Choose a reason for hiding this comment

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

@88250 修改过后台请求处理,为保持所有请求的一致性处理,这个没有其他情况的话请不要进行修改。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

用户在只读模式下打开设置面板后该请求会自动发出并返回异常, 因此需要额外处理一下

@@ -38,6 +38,8 @@ export class App {
registerServiceWorker(`${Constants.SERVICE_WORKER_PATH}?v=${Constants.SIYUAN_VERSION}`);
/// #endif
addBaseURL();
addScriptSync(`${Constants.PROTYLE_CDN}/js/lute/lute.min.js?v=${Constants.SIYUAN_VERSION}`, "protyleLuteScript"),
Copy link
Member

Choose a reason for hiding this comment

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

这一块比较复杂,没有必要请不要进行改动。如有必要的话请只修改关联部分即可。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这一块是为了与其他端调用顺序统一, 方便之后进行维护

@@ -507,8 +508,9 @@ export const exportMd = (id: string) => {
});
});
return;
} else if (response.code === 0) {
Copy link
Member

Choose a reason for hiding this comment

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

修改参见第一条

Copy link
Contributor Author

Choose a reason for hiding this comment

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

此处更改是为了避免出现异常但仍推送成功消息提示

@@ -37,9 +37,10 @@ class App {
if (!window.webkit?.messageHandlers && !window.JSAndroid) {
registerServiceWorker(`${Constants.SERVICE_WORKER_PATH}?v=${Constants.SIYUAN_VERSION}`);
}
addBaseURL();
Copy link
Member

Choose a reason for hiding this comment

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

请参见第二点进行修改

@@ -1676,6 +1677,7 @@ export class Gutter {
accelerator: window.siyuan.config.keymap.editor.general.quickMakeCard.custom,
iconHTML: '<svg class="b3-menu__icon" style="color:var(--b3-theme-primary)"><use xlink:href="#iconRiffCard"></use></svg>',
icon: "iconRiffCard",
disabled: window.siyuan.config.readonly,
Copy link
Member

Choose a reason for hiding this comment

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

相关的快捷键和其他菜单中的类似项也需要进行处理,须逐一核对。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

如何禁用相关的快捷键?

目前已找到的其他菜单中的类似项已在只读模式下禁用,但不排除还有未找到的菜单项

@@ -45,7 +45,7 @@ export const saveScroll = (protyle: IProtyle, getObject = false) => {

export const getDocByScroll = (options: {
Copy link
Member

Choose a reason for hiding this comment

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

这个方法就是通过 scrollAttr 打开的,传入如果为空就没有意义了。这种场景是?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

场景未知, 该问题是在发现控制台输出异常后定位到的
复现步骤: 只读/发布模式下刷新当前文档

@@ -150,8 +150,10 @@ export const initAssets = () => {
return;
}
}
window.siyuan.config.appearance = response.data.appearance;
loadAssets(response.data.appearance);
if (response.code === 0) {
Copy link
Member

Choose a reason for hiding this comment

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

参照第一点进行修改

@@ -27,9 +27,10 @@ class App {
public appId: string;

constructor() {
addBaseURL();
Copy link
Member

Choose a reason for hiding this comment

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

参照第二点进行修改

Copy link
Contributor Author

Choose a reason for hiding this comment

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

经过测试, 该方案正常工作, 且可以在 Constants.PROTYLE_CDN 变量发生更改时令脚本 /js/lute/lute.min.js/js/protyle-html.js 加载行为与其他地方加载的 /js/viewerjs/viewer.js 等脚本一致, 增强健壮性

@TCOTC
Copy link
Contributor

TCOTC commented Jun 20, 2024

啥时候能合并了发个 dev 版?

@Soltus
Copy link
Contributor

Soltus commented Jun 21, 2024

在线催更

@TCOTC
Copy link
Contributor

TCOTC commented Jun 22, 2024

@Zuoqiu-Yingyi
Copy link
Contributor Author

会禁用该菜单项

@JHPZ
Copy link

JHPZ commented Jun 24, 2024

我有个疑问,发布模式只能共享整个工作空间吗?假如我是Docker版,只有单个工作空间,它会把我想公布的和不想公布的都公布出去QAQ

@Zuoqiu-Yingyi
Copy link
Contributor Author

@Vanessa219
该 PR 已经挂起 10 天了, 有无新的进展?

@aptexd
Copy link

aptexd commented Jul 2, 2024

What happened? Is this still being tested?

@Jiangshuon
Copy link

盲猜作者大佬需按V大要求进行修改后才能合并O(∩_∩)O

@Vanessa219
Copy link
Member

看了下目前这个 PR 合并后还需要进行修改,等安排出修改的时间才能进行合并。

@Zuoqiu-Yingyi
Copy link
Contributor Author

看了下目前这个 PR 合并后还需要进行修改,等安排出修改的时间才能进行合并。

修改建议均进行了回复, 需要进一步讨论

@Vanessa219 Vanessa219 merged commit f25b36f into siyuan-note:dev Jul 5, 2024
Vanessa219 added a commit that referenced this pull request Jul 5, 2024
@aptexd
Copy link

aptexd commented Jul 5, 2024

Oh god, finally merged

@Zuoqiu-Yingyi Zuoqiu-Yingyi deleted the feat/publish branch July 5, 2024 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants