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

feat: add global theme switching config #2

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Aziteee
Copy link

@Aziteee Aziteee commented Nov 23, 2024

去除原先的代码块单独配置主题,添加了全局深浅色主题切换配置功能

image

image

增加全局主题切换配置

@f2c-ci-robot f2c-ci-robot bot added the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Nov 23, 2024
@f2c-ci-robot f2c-ci-robot bot requested review from guqing and JohnNiang November 23, 2024 03:54
Copy link

f2c-ci-robot bot commented Nov 23, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign ruibaby for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ruibaby ruibaby requested a review from LIlGG December 2, 2024 08:43
@f2c-ci-robot f2c-ci-robot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Dec 3, 2024
Copy link
Member

@LIlGG LIlGG left a comment

Choose a reason for hiding this comment

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

/lgtm

@f2c-ci-robot f2c-ci-robot bot added the lgtm Indicates that a PR is ready to be merged. label Dec 3, 2024
@LIlGG LIlGG requested a review from ruibaby December 3, 2024 07:34
Copy link
Member

@ruibaby ruibaby left a comment

Choose a reason for hiding this comment

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

感谢贡献,功能我测试没有问题,以下是一些改进建议。

@@ -111,6 +105,16 @@ export function ShikiPlugin({
// Initialize shiki async, and then highlight initial document
async initDecorations() {
const doc = view.state.doc;

let defaultTheme:BundledTheme = "github-light";
const configmap = await coreApiClient.configMap.getConfigMap({
Copy link
Member

Choose a reason for hiding this comment

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

这里推荐使用 consoleApiClient.plugin.plugin.fetchPluginJsonConfig()

此外,这里需要考虑权限问题,不是所有用户都能访问这个接口,个人中心的编辑器也需要使用。

我的建议是将这个接口附加给 role-template-view-posts(Console 文章查看权限)和 role-template-post-contributor(允许在个人中心投稿)。

可以参考:

Copy link
Author

Choose a reason for hiding this comment

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

fetchPluginJsonConfig用不了,报错Uncaught (in promise) TypeError: i.consoleApiClient.plugin.plugin.fetchPluginJsonConfig is not a function。这可能是一个bug

Copy link
Author

Choose a reason for hiding this comment

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

我改用fetchPluginConfig之后没问题了,但是参考示例写了角色模板后没用,能看看有啥地方出问题了吗

apiVersion: v1alpha1
kind: Role
metadata:
  name: role-template-shiki
  labels:
    halo.run/role-template: "true"
    halo.run/hidden: "true"
    rbac.authorization.halo.run/aggregate-to-role-template-view-posts: "true"
    rbac.authorization.halo.run/aggregate-to-role-template-post-contributor: "true"
  annotations:
    rbac.authorization.halo.run/module: "Shiki"
    rbac.authorization.halo.run/display-name: "Shiki"
rules:
  - apiGroups: [ "" ]
    resources: [ "configmaps" ]
    resourceNames: [ "shiki-configmap" ]
    verbs: [ "get" ]

Copy link
Member

Choose a reason for hiding this comment

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

fetchPluginJsonConfig用不了,报错Uncaught (in promise) TypeError: i.consoleApiClient.plugin.plugin.fetchPluginJsonConfig is not a function。这可能是一个bug

提升依赖(plugin.yaml#spec.requires)以及开发环境运行的 Halo 版本,fetchPluginJsonConfig 应该是在 2.20 才有

Copy link
Member

Choose a reason for hiding this comment

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

我改用fetchPluginConfig之后没问题了,但是参考示例写了角色模板后没用,能看看有啥地方出问题了吗

apiVersion: v1alpha1
kind: Role
metadata:
  name: role-template-shiki
  labels:
    halo.run/role-template: "true"
    halo.run/hidden: "true"
    rbac.authorization.halo.run/aggregate-to-role-template-view-posts: "true"
    rbac.authorization.halo.run/aggregate-to-role-template-post-contributor: "true"
  annotations:
    rbac.authorization.halo.run/module: "Shiki"
    rbac.authorization.halo.run/display-name: "Shiki"
rules:
  - apiGroups: [ "" ]
    resources: [ "configmaps" ]
    resourceNames: [ "shiki-configmap" ]
    verbs: [ "get" ]

ping @guqing

label: 配置
formSchema:
- $formkit: select
name: themeLight
Copy link
Member

Choose a reason for hiding this comment

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

可以考虑加上 searchable,支持搜索。

https://docs.halo.run/developer-guide/form-schema#select

label: 亮色主题
value: "vitesse-light"
<<: *theme-options
- $formkit: select
Copy link
Member

Choose a reason for hiding this comment

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

同上

.shiki > code {
display: block;
overflow-x: auto;
padding: 1em;
Copy link
Member

@ruibaby ruibaby Dec 4, 2024

Choose a reason for hiding this comment

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

建议为 .shiki 添加内边距,观察到不少主题为 pre 添加了边距样式,建议直接覆盖,否则可能造成 code 和 pre 都有内边距。

name: themeDark
label: 暗色主题
value: "vitesse-dark"
<<: *theme-options
Copy link
Member

Choose a reason for hiding this comment

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

学习了

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants