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

Refactor internal components into internal folder #3230

Closed
11 of 12 tasks
joshblack opened this issue Apr 26, 2023 · 7 comments · Fixed by #3369, #3370, #3371, #3372 or #3373
Closed
11 of 12 tasks

Refactor internal components into internal folder #3230

joshblack opened this issue Apr 26, 2023 · 7 comments · Fixed by #3369, #3370, #3371, #3372 or #3373
Assignees
Labels

Comments

@joshblack
Copy link
Member

joshblack commented Apr 26, 2023

This is a follow-up to our ADR on Internal Modules. This issues captures the remaining internal components or helpers that we should consider migrating into src/internal to prevent being imported outside of the package.

  • _CheckboxOrRadioGroup PR
  • _getGlobalFocusStyles PR
  • _InputCaption PR
  • _InputLabel PR
  • _InputValidation PR
  • _sharedCheckboxAndRadioStyles PR
  • _TextInputInnerAction PR
  • _TextInputInnerVisualSlot PR
  • _TextInputWrapper PR
  • _UnstyledTextInput PR
  • _ValidationAnimationContainer PR
  • _VisuallyHIdden
@lesliecdubs
Copy link
Member

Thanks for filing @joshblack! Should this issue go into our Primer teams backlog "inbox" for triage by the team, or did you have other plans for it?

@joshblack
Copy link
Member Author

Hey @lesliecdubs! No plans in particular, just was something that came up in PR reviews last week. Inbox triage could be a great place for it 👍

@green6erry green6erry linked a pull request May 8, 2023 that will close this issue
6 tasks
@green6erry green6erry linked a pull request Jun 5, 2023 that will close this issue
7 tasks
@green6erry green6erry linked a pull request Jun 5, 2023 that will close this issue
7 tasks
@green6erry green6erry linked a pull request Jun 5, 2023 that will close this issue
7 tasks
@green6erry green6erry linked a pull request Jun 5, 2023 that will close this issue
7 tasks
@green6erry green6erry linked a pull request Jun 5, 2023 that will close this issue
7 tasks
@green6erry green6erry linked a pull request Jun 5, 2023 that will close this issue
6 tasks
@green6erry green6erry linked a pull request Jun 5, 2023 that will close this issue
6 tasks
@green6erry green6erry reopened this Jun 5, 2023
@ollie-iterators
Copy link

ollie-iterators commented Jun 9, 2023

_VisuallyHidden.tsx is still on the page https://github.com/primer/react/tree/main/src, so it should not be checked off, but _TextInputInnerVisualSlot should be.

@green6erry
Copy link
Contributor

It appears there are two VisuallyHidden components. It looks like the main authors were @mperrotti on the one not in internal, and @joshblack for the one already in internal. The easiest way forward would be to keep both and they are used in multiple places. Is someone able to weigh in how to proceed?

@green6erry green6erry reopened this Jun 12, 2023
@mperrotti
Copy link
Contributor

We shouldn't have two - we should just have one.

@joshblack - does your VisuallyHidden do anything that the original doesn't?

@ollie-iterators
Copy link

It looks like the same thing also may be happening with _TextInputInnerAction.tsx

@joshblack
Copy link
Member Author

joshblack commented Jun 19, 2023

@mperrotti I think they have a difference in API and styles and we just haven't taken the time to merge them yet. I think the difference at that point ends up being that you would toggle rendering VisuallyHidden instead of passing in isVisible, if I remember right!

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