Added fix for issue #3867 - TextInput action hover state is incorrect

[cs25-esc]

This fix resolves :

1. Clear input icon will not be displayed when there is no input in the input box.
2. No background colour will be displayed for clear input icon.
3. Clear input icon colour slightly changes on hover (colours suggested by @maximedegreve )

https://github.com/primer/react/assets/68850280/eeafece0-b0c5-459b-b4cf-c265667585c1

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: da333c4c5db1e7a619019ca8e2ecaedeb8cda655

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

[siddharthkp]

Hi!

After discussing with @maximedegreve, I don't think fixing this just in the stories is the right approach here. We need to think about how we want to address this in the component itself 🤔

We can close this pull request and start with API. Sorry about that!

[cs25-esc]

Hi!

After discussing with @maximedegreve, I don't think fixing this just in the stories is the right approach here. We need to think about how we want to address this in the component itself 🤔

We can close this pull request and start with API. Sorry about that!

sure @siddharthkp no problem :)

[siddharthkp]

@cs25-esc, Gave it more thought, I think we can keep this PR!

Summarised the different parts of the problem here: https://github.com/primer/react/issues/3867#issuecomment-1798232181

The 2nd point is the only one relevant to this pull request. We should remove changeset and custom styles from the story

[cs25-esc]

@cs25-esc, Gave it more thought, I think we can keep this PR!

Summarised the different parts of the problem here: #3867 (comment)

The 2nd point is the only one relevant to this pull request. We should remove changeset and custom styles from the story

Hi @siddharthkp so you mean i can continue to work on this PR ?

[siddharthkp]

Hi @siddharthkp so you mean i can continue to work on this PR ?

Yes. but, we need to change the scope of this pull request.

From https://github.com/primer/react/issues/3867#issuecomment-1798232181,

Our trailing action example in storybook doesn't match our own design guidelines. This is incorrect behaviour and could encourage a wrong pattern. It's crucial that the clear button is hidden when the input is empty.

Because we want to show trailing action in the example, what we can do here is add a default value to the input to show the action and then hide it if you remove the text. To make this story even clearer, we can add two text inputs, one with text and one without.

[cs25-esc]

Hi @siddharthkp will this be helpful ?

A default value will be there whenever the page loads.

https://github.com/primer/react/assets/68850280/a7c967a5-946a-418a-92dc-59520a621a6a

[siddharthkp]

A default value will be there whenever the page loads.

Yep! I think that's the way to do it.

Looking at the video, it's a bit odd that the width of the input changes. Is there a way to make sure that doesn't happen? 🤔

[cs25-esc]

A default value will be there whenever the page loads.

Yep! I think that's the way to do it.

Looking at the video, it's a bit odd that the width of the input changes. Is there a way to make sure that doesn't happen? 🤔

yeah it happens due to the clear input icon if

A default value will be there whenever the page loads.

Yep! I think that's the way to do it.

Looking at the video, it's a bit odd that the width of the input changes. Is there a way to make sure that doesn't happen? 🤔

Like this ?

https://github.com/primer/react/assets/68850280/e19610a0-44c2-4f33-891d-523b04d7b5d0

[siddharthkp]

That looks much nicer!

[cs25-esc]

That looks much nicer!

Shall i make the changes then ?

[siddharthkp]

Yes, go for it 👍

[cs25-esc]

Yes, go for it 👍

i have a doubt like, i could see components WithTrailingAction and WithTooltipDirection in the same story share same implementation so shall i make changes for both ?

[cs25-esc]

Yes, go for it 👍

i have a doubt like, i could see components WithTrailingAction and WithTooltipDirection in the same story share same implementation so shall i make changes for both ?

ideally i need to make changes only for component WithTrailingAction

[siddharthkp]

i have a doubt like, i could see components WithTrailingAction and WithTooltipDirection in the same story share same implementation so shall i make changes for both ?

Yes please

[siddharthkp]

Hi @cs25-esc, I made some tiny changes (moved styles from story to component) to get a review on the visual feedback. Hope that's okay!

Note: The snapshot tests are failing, I will update them after getting a review on the hover colors :)

[github-actions[bot]]

Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days.