+
Skip to content
This repository was archived by the owner on Oct 14, 2025. It is now read-only.

Conversation

clintcs
Copy link
Collaborator

@clintcs clintcs commented Dec 19, 2024

🚀 Description

Added to CONTRIBUTING.md:

Avoid Lit's @query decorators

Avoid @query, @queryAll, and the like. We think refs are more natural—especially for those coming from React. And, unlike decorators, refs can be made private. So we can be sure they're only used internally.

When you can't use a ref because you need an element in the light DOM, use this.querySelector or this.querySelectorAll. If you need elements from a specific slot, use assignedElements().

📋 Checklist

🔬 How to Test

The tests should have us covered. But a poke around Storybook wouldn't hurt.

📸 Images/Videos of Functionality

N/A

@clintcs clintcs force-pushed the ditch-remaining-uses-of-@query branch from 74812e9 to d27334a Compare December 19, 2024 21:18
Copy link

changeset-bot bot commented Dec 19, 2024

🦋 Changeset detected

Latest commit: 1d55b9a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@crowdstrike/glide-core Minor

Not sure what this means? Click here to learn what changesets are.

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

slotElements!: GlideCoreTreeItem[];

@queryAssignedElements({ slot: 'suffix' })
suffixSlotAssignedElements!: HTMLElement[];
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unused.

Copy link
Contributor

And, [unlike](https://github.com/lit/lit/issues/4020#issuecomment-1743735312) the decorators, refs can be made private.
So we can be sure they're only used internally.

```ts
Copy link
Collaborator Author

@clintcs clintcs Dec 19, 2024

Choose a reason for hiding this comment

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

Rather than give examples, I thought we could defer to Lit's documentation—which is linked above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ynotdraw: Can we lint against @query decorators?

Copy link
Collaborator

@ynotdraw ynotdraw Dec 19, 2024

Choose a reason for hiding this comment

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

You bet! I'll add it to my list. Should be quick, I'll try and knock it out tomorrow.

@clintcs clintcs force-pushed the ditch-remaining-uses-of-@query branch 2 times, most recently from c972c39 to 02629be Compare December 19, 2024 21:29
`);

const childItems = component.querySelectorAll<TreeItem>(
':scope > glide-core-tree-item',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@clintcs clintcs force-pushed the ditch-remaining-uses-of-@query branch from 02629be to 7559530 Compare December 19, 2024 21:39
@clintcs clintcs force-pushed the ditch-remaining-uses-of-@query branch from 7559530 to 5369908 Compare December 19, 2024 21:42
@clintcs clintcs marked this pull request as ready for review December 19, 2024 21:45
src/tree.item.ts Outdated
);
}

// Checks if focus has moved to an element within this tree item itself,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unintentional comment shift?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Aye. Thank you.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@clintcs
Copy link
Collaborator Author

clintcs commented Dec 20, 2024

I've been seeing some flakiness with an Accordion test. The build failed because of it. Just pushed a commit that attempts to resolve it.

@clintcs clintcs merged commit ebd5137 into main Dec 20, 2024
7 checks passed
@clintcs clintcs deleted the ditch-remaining-uses-of-@query branch December 20, 2024 15:22
@github-actions github-actions bot mentioned this pull request Dec 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载