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(date picker) support month type #1202

Merged
merged 100 commits into from
Oct 6, 2021

Conversation

doom-9-zz
Copy link
Contributor

No description provided.

doom-9 and others added 30 commits June 17, 2021 17:42
@codecov
Copy link

codecov bot commented Sep 18, 2021

Codecov Report

Merging #1202 (b2f41b5) into main (47fde6b) will decrease coverage by 0.30%.
The diff coverage is 4.59%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1202      +/-   ##
==========================================
- Coverage   50.39%   50.09%   -0.31%     
==========================================
  Files         518      519       +1     
  Lines       12834    12919      +85     
  Branches     3649     3675      +26     
==========================================
+ Hits         6468     6472       +4     
- Misses       5270     5349      +79     
- Partials     1096     1098       +2     
Impacted Files Coverage Δ
src/date-picker/src/DatePicker.tsx 51.72% <0.00%> (-3.54%) ⬇️
src/date-picker/src/interface.ts 100.00% <ø> (ø)
src/date-picker/src/panel/month.tsx 0.00% <0.00%> (ø)
src/date-picker/src/utils.ts 48.48% <0.00%> (-21.09%) ⬇️
src/date-picker/src/panel/use-calendar.ts 31.31% <23.52%> (-0.83%) ⬇️

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 47fde6b...b2f41b5. Read the comment docs.

{{
default: () => [
...this.monthArray.map((monthItem, i) => (
<div
Copy link
Collaborator

Choose a reason for hiding this comment

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

我感觉 item render 的重复好像比较多,月份和年份有很多重复代码。把公共的部分抽象一下。

src/date-picker/src/utils.ts Show resolved Hide resolved
Copy link
Collaborator

@07akioni 07akioni left a comment

Choose a reason for hiding this comment

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

Validate 相关的事情也可以在 month 面板上考虑一下,is-date-disabled 应该要想办法作用上去。

@@ -448,6 +469,7 @@ export default defineComponent({
function openCalendar (): void {
if (mergedDisabledRef.value || mergedShowRef.value) return
doUpdateShow(true)
void nextTick(scrollTimer)
Copy link
Collaborator

Choose a reason for hiding this comment

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

if (props.type === 'month')

) : (
<DatePanel {...commonPanelProps} />
),

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

Comment on lines 46 to 47
monthScrollRef?: ScrollbarInst
yearScrollRef?: VirtualListInst
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
monthScrollRef?: ScrollbarInst
yearScrollRef?: VirtualListInst
monthScrollRef: ScrollbarInst | null
yearScrollRef: VirtualListInst | null

类型表现要尽量和 runtime 一致

},
render () {
const { mergedClsPrefix, mergedTheme, shortcuts } = this
const itemRenderer = (item: YearItem | MonthItem, i: number): VNode => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个看看能不提到 setup 里面,在 render 里面不好看

@@ -59,6 +85,28 @@ export interface DateItem {
ts: number
}

export interface MonthItem {
type: string
Copy link
Collaborator

Choose a reason for hiding this comment

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

type: 'month'

这种地方要精确,type guard 才会生效,不然 if else 分支都拿不到正确的类型

inCurrentMonth: boolean
selected: boolean
ts: number
showText?: string
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
showText?: string
formattedText?: string

Copy link
Collaborator

@07akioni 07akioni left a comment

Choose a reason for hiding this comment

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

整体看着还行,十一我调整一下。

给这个 month 的写俩测试吧。

主要是确保数据层面可以发出来对的数据,还有确保 disabled date 是可以生效的。

@07akioni 07akioni merged commit 3ea326e into tusen-ai:main Oct 6, 2021
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.

3 participants