feat(TextInputInnerAction): adds a tooltip-direction property to the …

[edersonlucas]

Component: TextInputInnerAction

In the TextInputInnerAction component, the tooltip direction property defaults to 'n'. Therefore, I decided to add the tooltip-direction property to the component.

Motivation

Recently, in a project I was working on, we had a bug in the mobile screen mode with a password input that had a button to show the password. When the button was clicked, the tooltip that appeared overflowed beyond the screen, causing a horizontal scroll. For this reason, I am adding the tooltip-direction property to the TextInputInnerAction.

Changelog

New

• Add tooltip-direction property.

Changed

• Update TextInput.docs.json to add the tooltip-direction property.
• Update the Storybook of the TextInput component to provide an example of how the tooltip-direction property would be used.

Removed

Screenshots

Before

After

Merge checklist

• Added/updated tests
• Added/updated documentation
• Tested in Chrome
• Tested in Firefox
• Tested in Safari
• Tested in Edge

[changeset-bot]

🦋 Changeset detected

Latest commit: 0ddad0c

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

This PR includes changesets to release 1 package

Name	Type
@primer/react	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

[broccolinisoup]

👋🏻 @edersonlucas

Thank you for taking the time to create this PR! The additional prop for the tooltip direction sounds good to me but I'm going to tag https://github.com/orgs/primer/teams/accessibility-design to see what they think. Thanks!

[edersonlucas]

@broccolinisoup Perfect, I'll be waiting for it.

[ichelsea]

Adding the extra prop of the direction adjustment shouldn't be a problem 🙂. It looks like we're using the updated accessible tooltip component (I think!) so that sounds good as well!

[broccolinisoup]

Thanks @ichelsea! The tooltip is still WIP for accessibility improvements https://github.com/github/primer/issues/1790 (GitHub stuff only link) hopefully should be completed soon 😊

[broccolinisoup]

@edersonlucas could you please add a changeset to the PR? https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md - feel free to have a look at the other PRs as well to get an idea of how we write changesets. Thanks!

[edersonlucas]

@broccolinisoup Thank you, I will analyze here how to make the changeset 😊

[edersonlucas]

@broccolinisoup I just made the changeset, I believe I did it correctly 😀

[edersonlucas]

@broccolinisoup Corrected 😀

[broccolinisoup]

Sorry for the late response! It looks good to me ✨ Thank you so much for your time to push that PR 🙏🏻

[edersonlucas]

@broccolinisoup Hello, I have a question. Is there any way I can fix this error in the "Deploy (fork) / Preview / Build (pull_request_target)" of Github Actions?

[broccolinisoup]

@edersonlucas Hi! I think they were expected to fail as this PR is coming from a fork but I'll confirm it with the team if it is okay to merge it with the fails. I'll get back to you tomorrow. Sorry for still not being able to merge this! Thanks for your patience 🙏🏻

[edersonlucas]

@broccolinisoup Ok, I understood. Thank you for the feedback 🙏🏻

[broccolinisoup]

@edersonlucas I think we are good to merge your PR! ✨

There is only one failing on the CI now and it is expected due to your PR is coming from a fork repo.
We fixed the other failing CI that was caused by peer dependency issues #3092

I am going to go ahead and merge your PR now. Thank you so much for your contribution! We really appreciate it 🙏🏻
Also for your info, we are currently working on remediating accessibility issues on the Tooltip component #3032 . Once it is released, TextInputInnerAction component will have an accessible tooltip as well ✨

[edersonlucas]

@broccolinisoup Hi! I was thinking, could this issue be because I directly modified the components.json? I say this because looking at other PRs, it's modified by Github Actions. Do you think if I revert it, it will pass the test?

[edersonlucas]

That's great, I'll take a look at #3032 and see if I can contribute in any way 😊

[broccolinisoup]

@broccolinisoup Hi! I was thinking, could this issue be because I directly modified the components.json? I say this because looking at other PRs, it's modified by Github Actions. Do you think if I revert it, it will pass the test?

It is failing because secrets (in this case, it is app_id) are not passed to workflows that are triggered by a pull request from a fork.
reference

[edersonlucas]

@broccolinisoup Thank you for your attention 🙏🏻 I recently started contributing to open-source projects here on GitHub and I would like to share the experience and insights I gained while contributing to Primer. I'm considering writing a post about it on LinkedIn. Would it be possible to mention you in my post?

[broccolinisoup]

@edersonlucas Thank you! You can mention me in your post. Would it be possible to share it with me before posting?

Also, it was brought to my attention that we have a pattern of using camel case prop names (tooltipDirection instead of tooltip-direction) and I totally missed this in your PR! Would you be keen to push another PR that renames the prop name to tooltipDirection ?

[edersonlucas]

@edersonlucas Thank you! You can mention me in your post. Would it be possible to share it with me before posting?

Also, it was brought to my attention that we have a pattern of using camel case prop names (tooltipDirection instead of tooltip-direction) and I totally missed this in your PR! Would you be keen to push another PR that renames the prop name to tooltipDirection ?

@broccolinisoup Thank you! Yes, I'll send it to you before posting. I'll look for you on LinkedIn.
Regarding the PR, I'll make the necessary corrections and submit another one 😊

[broccolinisoup]

@edersonlucas Thank you so much for your responsiveness. I appreciate it! You can find my linkedin on my github profile.

[broccolinisoup]

@edersonlucas We possibly need to do an urgent release so we wanted to make sure the tooltip direction prop is being released with the came case name. Please see the PR: #3101 We won't need another PR to rename the prop anymore. Thanks so much for your support and contribution on this work and sorry if there was any confusion!

[edersonlucas]

@edersonlucas We possibly need to do an urgent release so we wanted to make sure the tooltip direction prop is being released with the came case name. Please see the PR: #3101 We won't need another PR to rename the prop anymore. Thanks so much for your support and contribution on this work and sorry if there was any confusion!

Perfect! No need to apologize. I appreciate your attention and kindness once again!