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

SuperSelect - search position (HDS-4254) #2612

Merged
merged 12 commits into from
Jan 7, 2025

Conversation

KristinLBradley
Copy link
Contributor

@KristinLBradley KristinLBradley commented Dec 17, 2024

📌 Summary

If merged, this PR adds the searchFieldPosition to SuperSelect enabling SuperSelect::Multiple to display the search field within the before options area instead of in the trigger area.

Related:

🛠️ Detailed description

  • Updates ember-power-select version to 8.6.0 & above
  • Adds searchFieldPosition argument to SuperSelect components
    • Sets value of "before-options" for argument

📸 Screenshots

image

🔗 External links


👀 Component checklist

💬 Please consider using conventional comments when reviewing this PR.

Copy link

vercel bot commented Dec 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
hds-showcase ✅ Ready (Inspect) Visit Preview Jan 7, 2025 5:55pm
hds-website ✅ Ready (Inspect) Visit Preview Jan 7, 2025 5:55pm

@KristinLBradley KristinLBradley changed the title [DRAFT] SuperSelect - search position (HDS-4254) SuperSelect - search position (HDS-4254) Dec 19, 2024
@KristinLBradley KristinLBradley marked this pull request as ready for review December 19, 2024 16:49
@KristinLBradley KristinLBradley requested a review from a team as a code owner December 19, 2024 16:49
shleewhite
shleewhite previously approved these changes Dec 19, 2024
Copy link
Contributor

@shleewhite shleewhite left a comment

Choose a reason for hiding this comment

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

🚢

Copy link
Member

@alex-ju alex-ju left a comment

Choose a reason for hiding this comment

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

This seems to require a rebase to resolve some conflicts in yarn.lock, otherwise it's good to be merged.

…b.com/hashicorp/design-system into hds-4254-super-select-search-position

# Conflicts:
#	yarn.lock
@KristinLBradley
Copy link
Contributor Author

Resolved yarn.lock conflicts.

alex-ju
alex-ju previously approved these changes Jan 7, 2025
@KristinLBradley KristinLBradley dismissed alex-ju’s stale review January 7, 2025 09:23

The merge-base changed after approval.

alex-ju
alex-ju previously approved these changes Jan 7, 2025
@KristinLBradley KristinLBradley dismissed alex-ju’s stale review January 7, 2025 09:53

The merge-base changed after approval.

@@ -52,7 +52,7 @@
"ember-focus-trap": "^1.1.0",
"ember-get-config": "^2.1.1",
"ember-modifier": "^4.1.0",
"ember-power-select": "^8.2.0",
"ember-power-select": "^8.6.0",
Copy link
Member

Choose a reason for hiding this comment

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

we had two patch releases since we started the work on this one, so we should bump this to latest 8.6.2

Suggested change
"ember-power-select": "^8.6.0",
"ember-power-select": "^8.6.2",

Copy link
Member

Choose a reason for hiding this comment

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

of course, this would require a small adjustment to the changelog entry and a yarn.lock update

Copy link
Member

@alex-ju alex-ju Jan 7, 2025

Choose a reason for hiding this comment

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

on a second thought, we have a version bump in #2625 (review) as well, so you can get this in as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So is this one ok to just merge now as is?

Copy link
Member

Choose a reason for hiding this comment

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

Yes.
However, we're having issues with getting it approved. They automatically get dismissed after the main was merged into it. Would you be able to force-push from local to see if we can overcome this?

@alex-ju alex-ju mentioned this pull request Jan 7, 2025
2 tasks
alex-ju
alex-ju previously approved these changes Jan 7, 2025
@KristinLBradley KristinLBradley dismissed alex-ju’s stale review January 7, 2025 11:58

The merge-base changed after approval.

# Conflicts:
#	showcase/package.json
#	yarn.lock
@hashibot-hds hashibot-hds removed packages/flight-icons packages/ember-flight-icons docs-website Content updates to the documentation website labels Jan 7, 2025
@KristinLBradley KristinLBradley merged commit 2164713 into main Jan 7, 2025
14 checks passed
@KristinLBradley KristinLBradley deleted the hds-4254-super-select-search-position branch January 7, 2025 18:06
@hashibot-hds hashibot-hds mentioned this pull request Jan 7, 2025
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