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 Timeline.Item.position (#16148) #16193

Merged
merged 3 commits into from Apr 27, 2019

Conversation

MrHeer
Copy link
Contributor

@MrHeer MrHeer commented Apr 19, 2019

🤔 This is a ...

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

👻 What's the background?

close #16177 and #16148

💡 Solution

reuse 'alternate' less

📝 Changelog

add Timeline.Item.position

☑️ 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/index.en-US.md
View rendered components/timeline/index.zh-CN.md

@netlify
Copy link

netlify bot commented Apr 19, 2019

Deploy preview for ant-design ready!

Built with commit 52a1370

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

@MrHeer MrHeer changed the title Feature issues#16148 feat: Add Timeline.Item.position (#16148) Apr 19, 2019
@@ -12,7 +12,7 @@ export interface TimelineProps {
pendingDot?: React.ReactNode;
style?: React.CSSProperties;
reverse?: boolean;
mode?: 'left' | 'alternate' | 'right';
mode?: 'left' | 'alternate' | 'right' | 'customize';
Copy link
Member

Choose a reason for hiding this comment

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

No need to add mode="customize", just follow postion of item in higher priority.

Copy link
Contributor Author

@MrHeer MrHeer Apr 20, 2019

Choose a reason for hiding this comment

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

That's good idea

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the mode shuld be in higher priority.

@afc163
Copy link
Member

afc163 commented Apr 22, 2019

You should pull and rebase feature branch to fix ci.

@MrHeer
Copy link
Contributor Author

MrHeer commented Apr 22, 2019

You should pull and rebase feature branch to fix ci.

I did rebase, I'll try again tonight. It is necessary to rewrite history(Squash bd925d4 and e28570b)?

@afc163
Copy link
Member

afc163 commented Apr 22, 2019

Not necessary but better~

@MrHeer
Copy link
Contributor Author

MrHeer commented Apr 22, 2019

Thank you very much.

@codecov
Copy link

codecov bot commented Apr 22, 2019

Codecov Report

Merging #16193 into feature will decrease coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           feature   #16193      +/-   ##
===========================================
- Coverage    95.29%   95.25%   -0.05%     
===========================================
  Files          255      255              
  Lines         6824     6826       +2     
  Branches      1972     1949      -23     
===========================================
- Hits          6503     6502       -1     
- Misses         320      323       +3     
  Partials         1        1
Impacted Files Coverage Δ
components/timeline/TimelineItem.tsx 90.9% <ø> (ø) ⬆️
components/timeline/Timeline.tsx 100% <100%> (ø) ⬆️
components/_util/wave.tsx 85.57% <0%> (-2.89%) ⬇️

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 30a798c...52a1370. Read the comment docs.

@@ -58,6 +60,7 @@ const TimelineItem: React.SFC<TimeLineItemProps> = props => (
TimelineItem.defaultProps = {
color: 'blue',
pending: false,
position: 'left',
Copy link
Member

Choose a reason for hiding this comment

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

should follow <Timeline mode="xxx">, don't set in defaultProps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After the mode is set, the position will be invalid.

: '',
: ele.props.position === 'right'
? `${prefixCls}-item-right`
: `${prefixCls}-item-left`,
Copy link
Member

Choose a reason for hiding this comment

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

这段代码原本就挺看不懂,现在更难懂了。抽出来吧。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think so

@MrHeer MrHeer force-pushed the feature-issues#16148 branch 3 times, most recently from fb86a1c to 7b563b1 Compare April 23, 2019 12:55
@MrHeer
Copy link
Contributor Author

MrHeer commented Apr 25, 2019

@zombieJ I have modified. Is there other problem?

@zombieJ
Copy link
Member

zombieJ commented Apr 25, 2019

@MrHeer
Copy link
Contributor Author

MrHeer commented Apr 25, 2019

Yes, I have to change the style to accommodate that you can set the position of the dot in the case of default. When you set mode, the position will be invalid.

@zombieJ
Copy link
Member

zombieJ commented Apr 25, 2019

We should not break origin behavior.

@MrHeer
Copy link
Contributor Author

MrHeer commented Apr 25, 2019

Yeah, I understand, so I started to consider adding a mode customize (#16148). but afc163 didn't think it is necessary (just follow postion of item in higher priority.)

I think you have to add a new mode except you have a good plan

@afc163
Copy link
Member

afc163 commented Apr 25, 2019

We can use mode="alternate" as the custom mode.

@MrHeer
Copy link
Contributor Author

MrHeer commented Apr 25, 2019

We can use mode="alternate" as the custom mode.

Haha, nice, I also consider it, but feel a bit strange, so I add a customize(as same as alternate) mode.

I'll be commit tonight!!

@MrHeer
Copy link
Contributor Author

MrHeer commented Apr 25, 2019

I've updated, if you have time, please review it. Thank you!

@@ -41,3 +41,4 @@ Node of timeline
| -------- | ----------- | ---- | ------- |
| color | Set the circle's color to `blue`, `red`, `green` or other custom colors | string | `blue` |
| dot | Customize timeline dot | string\|ReactNode | - |
| position | Customize node position | `left` \| `right` | - |
Copy link
Member

Choose a reason for hiding this comment

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

One more column about avaliable version:
屏幕快照 2019-04-26 上午10 56 36

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I'll update tonight

Copy link
Contributor Author

Choose a reason for hiding this comment

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

version should be 3.16.6?

Copy link
Member

Choose a reason for hiding this comment

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

feature branch always released with minor version. Use 3.17.0 instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you

@afc163 afc163 merged commit 6cb76a9 into ant-design:feature Apr 27, 2019
@MrHeer MrHeer deleted the feature-issues#16148 branch April 27, 2019 11:38
chenshuai2144 added a commit that referenced this pull request May 6, 2019
* use ul in list

* update snapshot

* update comment

* feat: TreeSelect support `showSearch` in multiple mode (#15933)

* update rc-tree-select

* typo

* update desc & snapshot

* update desc & snapshot

* check default showSearch

* feat: table customizing variable (#15971)

* feat: added table selected row color variable

* fix: @table-selected-row-color default is inherit

* feat: Upload support customize previewFile (#15984)

* support preview file

* use promise

* dealy load

* use canvas of render

* use domHook of test

* update demo

* add snapshot

* update types

* update testcase

* feat: form customizing variables (#15954)

* fix: added styling form input background-color

* feat: added '@form-warning-input-bg' variable

* feat: added '@form-error-input-bg' variable

* use li wrap with comment

* feat: Support append theme less file with less-variable (#16118)

* add override

* add override support

* update doc

* feat: dropdown support set right icon

* docs: update doc of dropdown component

* style: format dropdown-button.md

* test: update updateSnapshot

* style: format dropdown-button.md

* test: update updateSnapshot

* test: update updateSnapshot

* style: change style of dropdown-button demo

* fix: fix document table order

* feat: Support SkeletonAvatarProps.size accept number (#16078) (#16128)

* chore:update style of demo

* feat: Notification functions accept top, bottom and getContainer as arguments

* drawer: add afterVisibleChange

* rm onVisibleChange

* update

* feat: 🇭🇷 hr_HR locale (#16258)

* Added Croatian locale

* fixed lint error

* ✅ Add test cases for hr_HR

* 📝 update i18n documentation

* feat:  add `htmlFor` in Form.Item (#16278)

* add htmlFor in Form.Item

* update doc

* feat: Button support `link` type (#16289)

close #15892

* feat: Add Timeline.Item.position (#16148) (#16193)

* fix: Timeline.pendingDot interface documentation there is a small problem (#16177)

* feat: Add Timeline.Item.position (#16148)

* doc: add version infomation for Timeline.Item.position

* refactor: Update Tree & TreeSelect deps (#16330)

* use CSSMotion

* update snapshot

* feat: Collapse support `expandIconPosition` (#16365)

* update doc

* support expandIconPosition

* update snapshot

* feat: Breadcrumb  support DropDown (#16315)

* breadcrumbs support drop down menu

* update doc

* add require less

* fix test

* fix md doc

* less code

* fix  style warning

* update snap

* add children render test

* feat: TreeNode support checkable

* feat: add optional to support top and left slick dots (#16186) (#16225)

* add optional to support top and left slick dots

* update carousel snapshot

* Update doc, add placement demo

* update carousel placement demo snapshots

* rename dots placement to position

* update vertical as deprecated

* rename dotsPosition to dotPosition

* refine code

* add warning testcase for vertical

* remove unused warning

* update expression

* Additional test case for dotPosition

* refactor: Upgrade `rc-tree-select` to support pure React motion (#16402)

* upgrade `rc-tree-select`

* update snapshot

* 3.17.0 changelog

* fix warning

* fix review warning
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