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

docs: refactor the layout closer to the official document #100

Merged
merged 21 commits into from May 10, 2020

Conversation

ElderJames
Copy link
Member

fix #95

@ElderJames
Copy link
Member Author

/preview

@github-actions
Copy link

github-actions bot commented May 9, 2020

@lindexi
Copy link
Contributor

lindexi commented May 10, 2020

和你推荐一个很好用的机器人,代码审查代码风格机器人 https://github.com/marketplace/codefactor 可以自动审查代码里面风格不符合,包括换行空格,以及静态代码漏洞例如资源没释放,值可能是空

@@ -63,6 +66,7 @@ protected void SetClassMap()
ClassMapper.Clear()
.Add("ant-btn")
.If($"{prefixName}-{this.Type}", () => !string.IsNullOrEmpty(Type))
.If($"{prefixName}-danger", () => Danger)
Copy link
Contributor

Choose a reason for hiding this comment

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

看起来代码没对齐

}

private string[] badgePresetColors =
Copy link
Contributor

Choose a reason for hiding this comment

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

推荐字段使用下划线开始

private IList<AntSubMenu> _openedSubMenus = new List<AntSubMenu>();
public void SelectItem(AntMenuItem item)
{
foreach (AntMenuItem menuitem in MenuItems.Where(x => x != item))
Copy link
Contributor

Choose a reason for hiding this comment

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

menu item 是两个词语,推荐使用 menuItem

base.OnInitialized();

if (InternalMode != AntMenuMode.Inline && _collapsed)
throw new ArgumentException($"{nameof(AntMenu)} in the {Mode} mode cannot be {nameof(Collapsed)}");
Copy link
Contributor

Choose a reason for hiding this comment

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

推荐告诉开发者,什么值才是符合预期的

@@ -24,30 +27,30 @@

- 🌈 提炼自企业级中后台产品的交互语言和视觉风格。
- 📦 开箱即用的高质量 Razor 组件,可在多种托管方式共享。
- 💕 支持基于 WebAssembly 的客户端和基于 SignalR 的服务端 UI 事件交互
- 💕 支持基于 WebAssembly 的客户端和基于 SignalR 的服务端UI事件交互
Copy link
Contributor

Choose a reason for hiding this comment

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

我认为 UI 这个单词前后需要添加空格

@@ -57,15 +60,15 @@

## 🎨 设计规范

与 Ant Design 设计规范定期同步,你可以在线查看[同步日志](https://github.com/ant-design-blazor/ant-design-blazor/actions?query=workflow%3A%22Style+sync+Bot%22)。
与 Ant Design 设计规范定期同步,你可以在线查看[同步日志](https://github.com/ElderJames/ant-design-blazor/actions?query=workflow%3A%22Style+sync+Bot%22)。
Copy link
Contributor

Choose a reason for hiding this comment

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

这里不是 ant-design-blazor 组织?

Suggested change
与 Ant Design 设计规范定期同步,你可以在线查看[同步日志](https://github.com/ElderJames/ant-design-blazor/actions?query=workflow%3A%22Style+sync+Bot%22)
与 Ant Design 设计规范定期同步,你可以在线查看[同步日志](https://github.com/ant-design-blazor/ant-design-blazor/actions?query=workflow%3A%22Style+sync+Bot%22)

```
$ git clone git@github.com:ant-design-blazor/ant-design-blazor.git
$ git clone git@github.com:ElderJames/ant-design-blazor.git
Copy link
Contributor

Choose a reason for hiding this comment

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

这里是不是需要修改为组织名?将 ElderJames 替换为 ant-design-blazor 呢

@@ -117,13 +118,14 @@

## 🗺 开发路线

查看 [这个 issue](https://github.com/ant-design-blazor/ant-design-blazor/issues/21) 来了解我们 2020 年的开发计划。
查看 [这个 issue](https://github.com/ElderJames/ant-design-blazor/issues/21) 来了解我们 2020 年的开发计划。
Copy link
Contributor

Choose a reason for hiding this comment

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

这里是不是需要修改为组织名?将 ElderJames 替换为 ant-design-blazor 呢


如果你希望参与贡献,欢迎 [Pull Request](https://github.com/ant-design-blazor/ant-design-blazor/pulls),或给我们 [报告 Bug](https://github.com/ant-design-blazor/ant-design-blazor/issues/new) 。
如果你希望参与贡献,欢迎 [Pull Request](https://github.com/ElderJames/ant-design-blazor/pulls),或给我们 [报告 Bug](https://github.com/ElderJames/ant-design-blazor/issues/new) 。
Copy link
Contributor

Choose a reason for hiding this comment

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

是否需要将这个文档全局将 ElderJames 替换为 ant-design-blazor 呢

string configFileDirectory = Path.Combine(Directory.GetCurrentDirectory(), output);
if (!Directory.Exists(configFileDirectory))
{
Directory.CreateDirectory(configFileDirectory);
Copy link
Contributor

Choose a reason for hiding this comment

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

在 Directory.CreateDirectory 内部判断了文件夹是否存在,所以放心调用,不需要先判断文件不存在


if (string.IsNullOrEmpty(docsDir) || !Directory.Exists(docsDir))
{
Console.WriteLine("Invalid docsDir.");
Copy link
Contributor

Choose a reason for hiding this comment

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

这里有点迷的是,如果用户传入的是相对路径,但是工作目录不对,例如这个应用是被其他应用调起的,此时调试的时候将会比较坑。如我本地测试传入 xx\xx 是对的,但是服务器测试就是炸了,这是为什么?一个原因是相对路径会根据工作路径决定绝对路径,而此时的错误信息里面只是输出 Invalid docsDir. 此时很难猜到是为什么

推荐的写法是 docsDir = System.IO.Path.GetFullPath(docsDir ) 输出的时候是 Console.WriteLine("docsDir = {docsDir } not found.");

private void GenerateFiles(string demoDirectory, string docsDirectory, string output)
{
DirectoryInfo demoDirectoryInfo = new DirectoryInfo(demoDirectory);
if (demoDirectoryInfo.Attributes != FileAttributes.Directory)
Copy link
Contributor

Choose a reason for hiding this comment

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

印象中这个判断是有坑的,应该通过 Directory.Exists 判断才比较稳哈

这个属性的判断需要使用 HasFlag 或 & 的方法,不能使用相等判断,原因是这个特性可以包括 是文件夹同时是压缩文件夹,此时的值就不等于 FileAttributes.Directory 了


File.WriteAllText(configFilePath, json);
Console.WriteLine("Generate demo file to {0}", configFilePath);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

这个方法太长了

}
catch (Exception e)
{
Console.WriteLine($"An error occurred. {e.Message}");
Copy link
Contributor

Choose a reason for hiding this comment

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

此时推荐使用 e.ToString 方法,这里输出的信息更全

Console.ForegroundColor = currentForegroundColor;
}

public static void WriteWelcome()
Copy link
Contributor

Choose a reason for hiding this comment

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

这个好像是多余的方法

{
private readonly PlatformInformationArbiter _platformInformation;

public string TmpDirectory => _platformInformation.GetValue(
Copy link
Contributor

Choose a reason for hiding this comment

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

好奇为什么不用 Path.GetTempPath() 代替

{
public class ConcurrentCache<TKey, TValue>
{
private readonly ConcurrentDictionary<TKey, Lazy<TValue>> dictionary;
Copy link
Contributor

Choose a reason for hiding this comment

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

推荐字段使用下划线开头

{
get
{
if (dictionary.ContainsKey(key))
Copy link
Contributor

Choose a reason for hiding this comment

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

注意,这句话是不对的。你在 1 线程判断存在,在你准备调用 dictionary[key].Value 返回的时候,在 2 线程更改删除了这个内容

Copy link
Contributor

Choose a reason for hiding this comment

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

请使用 TryGetValue 代替

@ElderJames ElderJames merged commit e19a0b7 into master May 10, 2020
@ElderJames ElderJames deleted the docs/refactor branch May 10, 2020 07:42
ElderJames added a commit that referenced this pull request Oct 15, 2020
* docs: add cli for building demo structured file

* docs: cli improvement

* docs: generate demo page

* fix: style of components page

* docs: add prism to mack source code highlight

* feat: add menu2json command for cli

* fix: markdown highlight

* feat: fetch menu data from cli output files

* fix: cli

* docs: add avatar demo

* docs: add button demos

* docs: add component-scope style

* docs: fix style

* docs: add badge demos

* fix: rebase conflict

* docs: refactor layout

* docs: fix navigation

* docs: fix rebase conflict

* docs: add AntAffix for sider menu

* docs: fix affix error

* docs: fix rebase confilct
ElderJames added a commit that referenced this pull request Apr 23, 2022
* docs: add cli for building demo structured file

* docs: cli improvement

* docs: generate demo page

* fix: style of components page

* docs: add prism to mack source code highlight

* feat: add menu2json command for cli

* fix: markdown highlight

* feat: fetch menu data from cli output files

* fix: cli

* docs: add avatar demo

* docs: add button demos

* docs: add component-scope style

* docs: fix style

* docs: add badge demos

* fix: rebase conflict

* docs: refactor layout

* docs: fix navigation

* docs: fix rebase conflict

* docs: add AntAffix for sider menu

* docs: fix affix error

* docs: fix rebase confilct
ElderJames added a commit that referenced this pull request Apr 30, 2022
* docs: add cli for building demo structured file

* docs: cli improvement

* docs: generate demo page

* fix: style of components page

* docs: add prism to mack source code highlight

* feat: add menu2json command for cli

* fix: markdown highlight

* feat: fetch menu data from cli output files

* fix: cli

* docs: add avatar demo

* docs: add button demos

* docs: add component-scope style

* docs: fix style

* docs: add badge demos

* fix: rebase conflict

* docs: refactor layout

* docs: fix navigation

* docs: fix rebase conflict

* docs: add AntAffix for sider menu

* docs: fix affix error

* docs: fix rebase confilct
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.

There is a bug in the sidebar of the demo ...
2 participants