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(comp:stepper): add dot prop #1401

Merged
merged 2 commits into from Jan 12, 2023
Merged

feat(comp:stepper): add dot prop #1401

merged 2 commits into from Jan 12, 2023

Conversation

kovsu
Copy link
Member

@kovsu kovsu commented Jan 11, 2023

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our guidelines
  • Tests for the changes have been added/updated or not needed
  • Docs and demo have been added/updated or not needed

What is the current behavior?

What is the new behavior?

image

Other information

@idux-bot
Copy link

idux-bot bot commented Jan 11, 2023

This preview will be available after the AzureCI is passed.

@kovsu
Copy link
Member Author

kovsu commented Jan 11, 2023

image

这是没有加 labelPlacement 属性的时候,显示的样子。

@codecov
Copy link

codecov bot commented Jan 11, 2023

Codecov Report

Merging #1401 (d96cd7b) into main (0a8b116) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1401   +/-   ##
=======================================
  Coverage   92.89%   92.90%           
=======================================
  Files         325      325           
  Lines       29919    29931   +12     
  Branches     2494     2495    +1     
=======================================
+ Hits        27794    27806   +12     
  Misses       2125     2125           
Impacted Files Coverage Δ
packages/components/stepper/src/StepperItem.tsx 96.15% <100.00%> (+0.15%) ⬆️
packages/components/stepper/src/types.ts 100.00% <100.00%> (ø)
packages/components/badge/src/types.ts 100.00% <0.00%> (ø)
packages/components/badge/src/Badge.tsx 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@danranVm
Copy link
Member

danranVm commented Jan 12, 2023

dot 的话还需要支持一下 slot, 把 key 和 status 作为插槽的参数

image

另外,你的 commit 里说的 support css style 是指?

Copy link
Member

@danranVm danranVm left a comment

Choose a reason for hiding this comment

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

另外记得更新一下测试快照, 不然 CI 过不了。

return normalizeClass({
[prefixCls]: true,
[`${prefixCls}-${status.value}`]: true,
[`${prefixCls}-active`]: isActive.value,
[`${prefixCls}-clickable`]: parentProps.clickable && !disabled,
[`${prefixCls}-disabled`]: disabled,
[`${prefixCls}-with-icon`]: icon || !!slots.icon,
[`${prefixCls}-dot`]: dot,
[`${prefixCls}-label-${labelPlacement}`]: true,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[`${prefixCls}-label-${labelPlacement}`]: true,

这个似乎不用加 ? 父组件已经有这个 class 了。

Copy link
Member Author

Choose a reason for hiding this comment

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

这个似乎不用加 ? 父组件已经有这个 class 了。

我加这个是想着 css 里面可以嵌套在 那个 .ix-stepper-item 里面写,就不用去 .ix-stepper 里面去写。然后我用 npm run test -u,它好像更新了所有测试的快照

Copy link
Member

Choose a reason for hiding this comment

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

更新了所有测试的快照?是不是你的依赖版本比较低? 删除 node-modules 和 pnpm-lock 文件重新 install 一下再跑一下试试呢?

Copy link
Member Author

Choose a reason for hiding this comment

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

了所有测试的快照?是不是你的依赖版本比较低? 删除 node-modules 和

ok,我去试一下

Copy link
Member

Choose a reason for hiding this comment

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

命令应该是 npm run test -- -u

@kovsu kovsu changed the title feat(comp:stepper): add dot prop and support css style feat(comp:stepper): add dot prop Jan 12, 2023
@kovsu
Copy link
Member Author

kovsu commented Jan 12, 2023

dot 的话还需要支持一下 slot, 把 key 和 status 作为插槽的参数

image

其实我不太能理解这里为什么要在 ix-stepper 里面使用插槽,不清楚要具体做什么。你可以和我说一下吗,谢谢 😂。

@danranVm
Copy link
Member

dot 的话还需要支持一下 slot, 把 key 和 status 作为插槽的参数
image

其实我不太能理解这里为什么要在 ix-stepper 里面使用插槽,不清楚要具体做什么。你可以和我说一下吗,谢谢 😂。

最开始的想法是可以用一个插槽搞定自定义所有的点(根据 Key 的不同去),而不用在每个 Item 里都写一遍

  <IxStepper>
    <IxStepperItem title="Finished"></IxStepperItem>
    <IxStepperItem title="In Progress"></IxStepperItem>
    <IxStepperItem title="Waiting"></IxStepperItem>
    
     <template #dot={key}>
         <div v-if="key===1"></div>
     </template>
  </IxStepper>

不过刚刚想了下,似乎确实也不太合理,暂时就不支持插槽吧,你把文档改一下吧,删掉 #dot={key, status}.

@danranVm
Copy link
Member

@kovsu 测试快照的话,上面有回复你。
image

@kovsu
Copy link
Member Author

kovsu commented Jan 12, 2023

@kovsu 测试快照的话,上面有回复你。 image

image
你看我运行 npm run test -- -u 也是这样,我也删掉 lock 文件和依赖重新安装了。但是他还是更新了全部组件的测试快照

@danranVm
Copy link
Member

可能是因为 windows 系统的文件结束符导致的?

你就把你 add 的这两个文件提交吧。

@kovsu
Copy link
Member Author

kovsu commented Jan 12, 2023

可能是因为 windows 系统的文件结束符导致的?

你就把你 add 的这两个文件提交吧。

ok 谢谢了啊😀

@danranVm danranVm merged commit aa90a7e into IDuxFE:main Jan 12, 2023
@liuzaijiang
Copy link
Contributor

可能是因为 windows 系统的文件结束符导致的?
你就把你 add 的这两个文件提交吧。

ok 谢谢了啊😀

git config --global core.autocrlf input
git config --global core.safecrlf true

@kovsu
Copy link
Member Author

kovsu commented Jan 12, 2023

好的,我配置一下。谢谢 : -)

@kovsu kovsu deleted the stepper-dot branch January 12, 2023 12:00
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