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 umi ui #378

Merged
merged 6 commits into from Nov 20, 2020
Merged

Feat umi ui #378

merged 6 commits into from Nov 20, 2020

Conversation

bluescurry
Copy link
Contributor

  1. 修改 dumi 关于对接 umi-ui 的文档错误
  2. applyCodeBlock 方法中增加 previewUrl 参数用来设置预览 url 地址

@github-actions
Copy link

github-actions bot commented Nov 11, 2020

😭 Deploy PR Preview 790dd1f failed. Build logs

🤖 By surge-preview

@github-actions
Copy link

github-actions bot commented Nov 11, 2020

🎊 PR Preview 790dd1f has been successfully built and deployed to https://umijs-dumi-preview-pr-378.surge.sh

🕐 Build time: 211.951s

🤖 By surge-preview

@codecov
Copy link

codecov bot commented Nov 11, 2020

Codecov Report

Merging #378 (790dd1f) into master (047684b) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #378   +/-   ##
=======================================
  Coverage   92.95%   92.95%           
=======================================
  Files         105      105           
  Lines        2059     2059           
  Branches      689      670   -19     
=======================================
  Hits         1914     1914           
  Misses        141      141           
  Partials        4        4           
Impacted Files Coverage Δ
...kages/preset-dumi/src/transformer/remark/jsxify.ts 100.00% <ø> (ø)
...es/preset-dumi/src/transformer/remark/previewer.ts 98.57% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 047684b...790dd1f. Read the comment docs.

Copy link
Member

@PeachScript PeachScript left a comment

Choose a reason for hiding this comment

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

感觉可以把 config.sitemap.hostname 拎出来?这样我们其实可以为用户直接生成 previewUrl

@@ -97,6 +97,7 @@ function applyCodeBlock(props: IPreviewerComponentProps, componentName: string)
description: props.description,
thumbnail: props.thumbnail,
tags: props.tags,
previewUrl: props.previewUrl || props.previewurl,
Copy link
Member

Choose a reason for hiding this comment

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

这里统一 previewUrl 吧,目前 code 属性的解析会把驼峰变成中横线,还没找到优雅的方式,得特殊处理下:https://github.com/umijs/dumi/blob/master/packages/preset-dumi/src/transformer/remark/jsxify.ts#L19

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我试了一下,code 属性设置 preview-url ,previewer.js 中拿到的是 props['preview-url'];设置 previewUrl,previewer.js 中拿到的是 props.previewurl。是不是和预期不太符合?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这里统一 previewUrl 吧,目前 code 属性的解析会把驼峰变成中横线,还没找到优雅的方式,得特殊处理下:https://github.com/umijs/dumi/blob/master/packages/preset-dumi/src/transformer/remark/jsxify.ts#L19

请问您看到我的回复了吗,非常感谢如果您能回复我,我认为预览这个功能很重要,用户可以提前体验到这个区块的所有功能,以确定这个区块是否是自己想要的

Copy link
Member

Choose a reason for hiding this comment

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

明白,我今天把 previewUrl 这个问题补进 PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

明白,我今天把 previewUrl 这个问题补进 PR

非常感谢😁

@bluescurry
Copy link
Contributor Author

感觉可以把 config.sitemap.hostname 拎出来?这样我们其实可以为用户直接生成 previewUrl

我认为是可行的。不过我觉得最好还是要支持自定义 previewUrl 配置~

@PeachScript
Copy link
Member

previewUrl 的特殊处理加上了,不过没太理解为何要把 previewUrl 传给 Previewer,应该只需要元数据里面有就够了

@bluescurry
Copy link
Contributor Author

previewUrl 的特殊处理加上了,不过没太理解为何要把 previewUrl 传给 Previewer,应该只需要元数据里面有就够了

我的想法是在创建 assets.json 时,为区块增加 previewUrl 属性,用于在 umi-ui 使用区块时的预览功能,我看源码的时候发现是在 previewer.ts 中进行处理的,所以我修改了 previewer.ts,您看是有我的理解有误吗?在 previewer.ts 做处理是正确的吗

@PeachScript
Copy link
Member

我的想法是在创建 assets.json 时,为区块增加 previewUrl 属性,用于在 umi-ui 使用区块时的预览功能,我看源码的时候发现是在 previewer.ts 中进行处理的,所以我修改了 previewer.ts,您看是有我的理解有误吗?在 previewer.ts 做处理是正确的吗

明白了,是我想岔了👍

@PeachScript PeachScript merged commit aad9651 into umijs:master Nov 20, 2020
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.

None yet

2 participants