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

fix(collapse): modify the disabled style #76

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

XiCheng148
Copy link

No description provided.

Copy link

changeset-bot bot commented Mar 18, 2024

⚠️ No Changeset found

Latest commit: 3108266

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@FriedRiceNoodles
Copy link
Owner

首先感谢PR,不过如果这个pr是为了解决 #63 的话,也许换一种修改方法会更合适一些。

上面那个issue的问题来源于collapse__header被css赋予了cursor: pointer,建议这样修改:

  1. collapse--disabled的位置保持原来那样不变
  2. 在css处使禁用状态下的collapse__header的cursor为not-allowed

建议这么修改的原因是:

  1. collapse--disabled这一class代表的是整个collapse的状态,不应该放在collapse__header上。
  2. 现在collapse组件的逻辑是,禁用状态下无论如何也无法使collapse变为展开状态,所以即使只把collapse--disabled放在header上也暂时没有问题;但是后续如果修改了组件的逻辑,允许展开和禁用状态同时存在,那禁用的样式就只会存在于header上而不存在于展开内容上了。

class="collapse__header"
class=${classMap({
collapse__header: true,
'collapse--disabled': this.disabled,
Copy link
Owner

Choose a reason for hiding this comment

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

名为collapse--disabled的class不应放在header上,详见comment。

Copy link
Author

Choose a reason for hiding this comment

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

了解了。
如果要实现 2 的效果这里父节点的cursor不会覆盖子节点的cursor,可能需要考虑更改一下position。

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.

2 participants