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: Add select on focus to text field #1072

Merged
merged 2 commits into from
Sep 23, 2024
Merged

Conversation

JonnCharpentier
Copy link
Contributor

No description provided.

@@ -61,6 +61,7 @@ export interface TextFieldBaseProps<X>
// Replaces empty input field and placeholder with node
// IE: Multiselect renders list of selected items in the input field
unfocusedPlaceholder?: ReactNode;
selectOnFocus?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @JonnCharpentier , just curious, but I'm getting back into things--what ticket/request was this change for? I'm wondering if we could add an in-source comment to this, like:

/** Disable select on focus, i.e. ...description of the use case you're supporting... */

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is for this ticket: https://app.shortcut.com/homebound-team/story/56770/improve-add-new-attribute-ux

Basically Dave want us to auto swap into the text field when the user search don't have matches, but also auto-focus the field for the user to continue typing into the text field, this change optionally removes the Select all on focus, so that when we swap into the text field, the field is focus but not selected.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, that makes sense -- the comment as-is does not really explain "why", imo it is just restating "what" happens; can you reword it to be:

/** Allow focusing without selecting, i.e. to let the user keep typing after we've pre-filled text + called focus, like the Add New component. */

@JonnCharpentier JonnCharpentier enabled auto-merge (squash) September 23, 2024 17:36
@JonnCharpentier JonnCharpentier merged commit 6cc4009 into main Sep 23, 2024
5 of 6 checks passed
@JonnCharpentier JonnCharpentier deleted the sc-56770/attribute-ux branch September 23, 2024 17:38
homebound-team-bot pushed a commit that referenced this pull request Sep 23, 2024
## [2.368.0](v2.367.0...v2.368.0) (2024-09-23)

### Features

* Add select on focus  to text field  ([#1072](#1072)) ([6cc4009](6cc4009))
@homebound-team-bot
Copy link

🎉 This PR is included in version 2.368.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

4 participants