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

add: timeline mode stamp #18035

Closed
wants to merge 4 commits into from
Closed

add: timeline mode stamp #18035

wants to merge 4 commits into from

Conversation

xrkffgg
Copy link
Member

@xrkffgg xrkffgg commented Aug 1, 2019

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / document update
  • Component style update
  • TypeScript definition update
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Other (about what?)

🔗 Related issue link

#17966

💡 Background and solution

img

📝 Changelog

Language Changelog
🇺🇸 English Add timeline mode stamp
🇨🇳 Chinese 增加时间戳模式,stamp-left 可将时间戳放在左侧,内容放在右侧。stamp-right 可将时间戳放在右侧,内容放在左侧

☑️ Self Check before Merge

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

View rendered components/timeline/demo/stamp.md
View rendered components/timeline/index.en-US.md
View rendered components/timeline/index.zh-CN.md

@netlify
Copy link

netlify bot commented Aug 1, 2019

Deploy preview for ant-design ready!

Built with commit f0aac11

https://deploy-preview-18035--ant-design.netlify.com

@xrkffgg xrkffgg mentioned this pull request Aug 1, 2019
14 tasks
@codecov
Copy link

codecov bot commented Aug 1, 2019

Codecov Report

Merging #18035 into feature will decrease coverage by 0.01%.
The diff coverage is 50%.

Impacted file tree graph

@@             Coverage Diff             @@
##           feature   #18035      +/-   ##
===========================================
- Coverage    96.12%    96.1%   -0.02%     
===========================================
  Files          271      271              
  Lines         7478     7480       +2     
  Branches      2093     2054      -39     
===========================================
+ Hits          7188     7189       +1     
- Misses         288      289       +1     
  Partials         2        2
Impacted Files Coverage Δ
components/timeline/Timeline.tsx 100% <ø> (ø) ⬆️
components/timeline/TimelineItem.tsx 84.61% <50%> (-6.3%) ⬇️

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 60c4316...f0aac11. Read the comment docs.

@buildsize
Copy link

buildsize bot commented Aug 2, 2019

File name Previous Size New Size Change
package-lock.json 861.79 KB 863.09 KB 1.3 KB (0%)

@xrkffgg
Copy link
Member Author

xrkffgg commented Aug 2, 2019

在原有 stamp 的基础上,新修改为 stamp-left stamp-right 模式,可控制时间戳的 展示位置。

new img

@xrkffgg
Copy link
Member Author

xrkffgg commented Aug 2, 2019

@afc163 大佬 这个

codecov/patch — 50% of diff hit (target 96.12%)

是啥意思??


ReactDOM.render(
<Timeline mode="stamp-left">
<Timeline.Item stamp="2015-09-01">Create a services site</Timeline.Item>
Copy link
Member

Choose a reason for hiding this comment

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

用 label 比较好,stmap 语义受限了。

感觉不需要有 mode 了。

Copy link
Member Author

Choose a reason for hiding this comment

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

ok

Copy link
Member Author

Choose a reason for hiding this comment

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

先修改了名称。
mode 的话,因为轴线改变,各个元素的位置动了。不用整体 mode 来控制,单个处理较复杂。暂时先这样,之后再想想。
😂😂😂

@xrkffgg
Copy link
Member Author

xrkffgg commented Aug 2, 2019

这一版 failed 的 2个 是其他 组件报的 😓 奇怪哟

@@ -12,7 +12,7 @@ export interface TimelineProps {
pendingDot?: React.ReactNode;
style?: React.CSSProperties;
reverse?: boolean;
mode?: 'left' | 'alternate' | 'right' | 'stamp-left' | 'stamp-right';
mode?: 'left' | 'alternate' | 'right' | 'label-left' | 'label-right';
Copy link
Member

Choose a reason for hiding this comment

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

我感觉这两个 mode 都没必要了。mode="left" + label === label-left

Copy link
Member

Choose a reason for hiding this comment

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

这个自动的比较好,没必要这么复杂,如果有 label 就根据 mode 当前的位置自动适应左右就好

Copy link
Member Author

@xrkffgg xrkffgg Aug 3, 2019

Choose a reason for hiding this comment

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

😂😂😂 感觉 这样处理起来有点 复杂呀
如果使用一个mode,单个 model 的样式 有两套。
按照现有的定义,需要修改 tail head head-customcontent
比如啊:

  • ant-timeline-item-right 的定义是在 Timeline.tsx 文件中
  • Timeline.tsx 是不太好获取 label 的值
  • 这样就使得 right 模式 和 label-right 模式 都使用的是 ant-timeline-item-right 的父 class
  • 这使得 在 TimelineItem.tsx 中 使用的是相同的父 class ,即 tail head content 按照现在的 class 是无法分开的。

要实现这个功能的话:

  • 1 最直接分开 mode, tail head head-customcontent 再定义一套

  • 2 无新 mode,tail head content 等 class 分2个,这样改动很大。

    • TimelineItem.tsx 需要判断 所有的 item 中,是否有一个 是有值的,才启动 lable 模式
    • 根据是否是 label 模式 来 定义两种 class 来适配 这4个 div
    • 现有元素不管是哪种 mode ,单个 div 样式保持 ant-timeline-item-tail 一致的,如果改动,就不能保证一致。

  • 3 无新 mode,tail head content 也使用现有的。但是需要自动适配 label 的有无。同样需要 循环判断 是否启动 label ,并且 现有的 4个div css 基本需要都改动。而且 改动 难度 都较大。

😅😅😅

Copy link
Member

Choose a reason for hiding this comment

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

ant-timeline-item-content 并列放置一个 ant-timeline-item-label,根据 ant-timeline-item-[right/left] 来定位 label 的位置

Copy link
Member Author

Choose a reason for hiding this comment

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

但是这个时候,tail head content 的位置 大小 都发生了变动 在一个ant-timeline-item-right 里,不太容易 进行2个定义吧

Copy link
Member Author

Choose a reason for hiding this comment

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

@shaodahong
还是有点疑问
如果

<Timeline>
    <Timeline.Item>Create a services site 2015-09-01</Timeline.Item>
    <Timeline.Item>Solve initial network problems 2015-09-01</Timeline.Item>
    <Timeline.Item>Technical testing 2015-09-01</Timeline.Item>
    <Timeline.Item label="2019-01-01">Network problems being solved 2015-09-01</Timeline.Item>
</Timeline>

像这样的,只在最后一个 item 写了 label,这种情况下,如果要启用 label 模式,则在 li 层 增加 ant-timeline-label 样式时,这就需要遍历完成后 才能判断 。

当前代码的 遍历 是在处理 每一个 item 的 样式
所以 我感觉 还是需要 2个 遍历啊

Copy link
Member Author

Choose a reason for hiding this comment

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

你 在上面说的 .some

是在每一个 item 去执行总的数组中去判断 是否 有一个有 label 吗?
这样 每一次 都执行个 some 不太好吧

Copy link
Member

Choose a reason for hiding this comment

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

恩。现在的代码已经拿到了要渲染的 childrens

const truthyItems = timeLineItems.filter(item => !!item);

再 some 一下

const isLabel = truthyItems.some(item => item.props.label !== undefined)

之后吧样式设置下

classNames({ [`${prefixCls}-label`]: isLabel })

Copy link
Member Author

Choose a reason for hiding this comment

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

😂 嗯,那就行。。。

之前 一直 想规避 这个 2次的遍历,还以为你们不接受 2次 遍历呢

Copy link
Member

Choose a reason for hiding this comment

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

我不知道官方接不接收,但是这个性能影响几乎忽略不计

@xrkffgg xrkffgg closed this Aug 3, 2019
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

3 participants