Code Review v1.0

[zxc0328]

HTML

• base.html里的meta里需要加SEO相关字段，以及各种favicon。
• 评论框那边为什么用了contenteditable？如果有什么收获的话应该在群里分享。

CSS

• 评论框（展开前和展开后）都有问题，水平布局一边用float一边用绝对定位这样，或者一边用float一边不用。水平布局应该如何做这个已经说了很多遍了。搜索框也是一样，水平布局就OK，没有必要用绝对定位。
• 表单元素应该加上规范的reset，不是所有浏览器的input的display都是inline-block的。
• z-index应该抽出来放在单独的文件里面作为变量引入。这样就可以统一管理z-index，不用设置非常大的z-index值。

JS

主要是组件抽象的问题，如果你在代码中发现了很多重复的代码，或者是发现一个文件有500行，很长，那就说明你需要抽象了。

• Fetch那边有很多重复的代码，应该把Fetch封装一下，接收URL和Method，还有一些参数，返回Promise。
• Ad、亮了、Banner、Modal（弹出框，比如要你登录那个）都应该抽成组件
• Search这里很有意思，你想的大概是Search拿到请求之后，然后用全局的bus传递数据，用首页的组件显示？组件的确是可以复用。通常的做法是新建一个search_result路由，搜索框里输入字段后（比如“运动会”），点击搜索，就会跳转到/search_result?key=运动会。然后search_result路由负责发起网络请求和渲染列表。
• profile这里，明显要把底下每一个板块都拆出组件。profile这个实在在太乱了，你自己应该有感觉的。

待续

[Elegenthus]

• Banner抽成组件
• 亮了抽成组件
  ad是一个写死在前端的链接跳转，我觉得不是很需要抽成一个单独的组件，所以没有抽出来。。。
• Fetch封装
• CSS

[Elegenthus]

• modal抽出,ad
• profile分离
  (6fd5cca)

[zxc0328]

css那个zindex的修改真是莫名其妙。你觉得这个页面里会有6个元素同时重叠的情况吗？需要这么多级吗？提取出来本来就是用尽量少的zindex值。然后放在color.scss里面也是很奇怪，不要将就啊，新建一个文件不就完了。

FetchData这个API的设计有问题，那个callback是做什么的，错误处理吗？这应该统一用Promise来搞。FetchData应该返回一个Promise。我说的是你自己new一个，不是直接返回fetch返回的Promise。你现在直接返回fetch的Promise的话，因为加了res.json()这种处理，所以只是针对成功情况。错误请求你只好用回调了。你这边搞这个的目的就是封装细节，直接返回fetch的东西本来就是不对的。这个本来就是需要你加一层抽象的。fetch成功的时候就resolve(res.json())，失败的时候就reject(error)。错误处理只要在FetchData返回的Promise后面then处理就行了。
还有要吐槽的就是你在import这个utility的时候变量名是FETCH。这种全大写一般是用来命名常量的，这个你应该清楚啊。你叫Fetch或者getJSON什么的都行。

你这commit还是有点乱，并不是和我提的问题一一对应的，我看的比较累。可以多commit几次也没事。

还有啊，这个项目的模板是经过预处理的，所以可以完全接入Webpack的工作流。比如你的favicon，是可以经过预处理的。具体你可以看favicons-webpack-plugin。生成的文件路径应该是可以ejs模板里的变量htmlWebpackPlugin.files里拿到的。
like this:

  <% if (htmlWebpackPlugin.files.favicon) { %>
  <link rel="shortcut icon" href="<%= htmlWebpackPlugin.files.favicon%>">
  <% } %>

favicon这种会有很多场景，比如iOS的添加到首屏图标，windows的瓷砖。一个尺寸压根不够用。

CSS的话评论框那里没改来着

@Elegenthus

[zxc0328]

5月11日 UI方面的Review

• 图片的话，每日一图那边那个svg icon为啥一直显示不了。文章下面的小桂表情建议直接作为CSS背景引入，webpack require进去。这样图片会变成inline base64，减少了请求。现在这几个图片的请求拖慢了整体的速度。
• 如果没有推荐的话，评论框会挡住页面的下部，广告位会被遮住一部分。要加个margin。
• 修改个人信息那边，点编辑，出现的文本框里原来的内容被设置为了placeholder，这个不对啊，应该是value才对。这样就可以直接在原来的基础上修改了。
• 我的收藏里那个列表，和顶部没有margin，我觉得要加边距。

[zxc0328]

我发现你浏览图标和边上的其他东西不是垂直居中的，就看了一下。

首页列表里的底部，浏览数标签那一栏，你在同一个元素上用了float，然后还用了display:inline-block。说了多少次了这两个是不同同时用的，因为float的元素会强制用BFC排版，相当于设置display:block，不管你display设置的是其他的什么，都不管用。你现在是需要水平排布，然后垂直居中，你就用display:inline-block就行了。如果你要用float，也是先分左右两块，用float，然后左右两块内部用display:inline-block。

[zxc0328]

首页列表的描述文字竟然用了webkit私有属性。你有考虑过兼容问题吗。私有属性一般情况下是迫不得已才用的。你现在要达到这种效果，首先后端返回的文字控制一下字数就行了。然后因为文字的font-size是固定的，你算一下最大高度也就行了。

[Elegenthus]

好的，收到

[Elegenthus]

• CSS评论页面
• 删除描述文字私有属性、添加喝茶吧、广告位下加margin
• 背景引入点亮图片、修改个人信息placeholder改value
• 添加天气图标
• 减少Zindex、首页信息流同时使用float&inline-block问题

[Elegenthus]

• 加入不同size的favicon