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(web): create tag on the fly #14726

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

MehringTing
Copy link

@bo0tzz bo0tzz changed the title add a new feat(web): create tag on the fly feat(web): create tag on the fly Dec 16, 2024
@alextran1502
Copy link
Contributor

Hello, thank you for the PR. Can you help fix the e2e test?

@MehringTing MehringTing force-pushed the b-feat-create-tag-on-the-fly branch 2 times, most recently from 5fa8885 to 34197f8 Compare December 17, 2024 09:23
let filteredOptions = $derived.by(() => {
const _options = options.filter((option) => option.label.toLowerCase().includes(searchQuery.toLowerCase()));

if (allowCreate && searchQuery !== '' && _options.filter((option) => option.label === searchQuery).length === 0) {
Copy link
Member

Choose a reason for hiding this comment

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

This filter doesn't do anything does it? You only want _options.length === 0

@@ -36,6 +36,8 @@
options?: ComboBoxOption[];
selectedOption?: ComboBoxOption | undefined;
placeholder?: string;
allowCreate?: boolean;
defaultFirstOption?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you help put some comment of what does this props do?

@@ -50,20 +55,22 @@
<FullScreenModal title={$t('tag_assets')} icon={mdiTag} onClose={onCancel}>
<div class="text-sm">
<p>
<FormatMessage key="tag_not_found_question">
<!-- <FormatMessage key="tag_not_found_question">
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove this unused code?

Copy link
Contributor

@alextran1502 alextran1502 left a comment

Choose a reason for hiding this comment

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

I just tested the PR and it works well.

Can you help address the comments? And what do you think about making a wrapper component for the combobox and adding specific tag creation handling in that wrapper component instead of adding those logics to the combobox?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants