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

[PBNTR-515] Remove margin bottom from Typeahead kit, un-revert PBNTR-479 #3680

Merged
merged 8 commits into from
Sep 20, 2024

Conversation

ElisaShapiro
Copy link
Contributor

@ElisaShapiro ElisaShapiro commented Sep 10, 2024

What does this PR do? A clear and concise description with your runway ticket url.
PBNTR-515 is attempt #2 at PBNTR-479, which had to be reverted from the Playbook 14.3.1 release due to an issue found in ninja testing where the Creatable-async-data typeahead used here (nitroqa link, component code link) did not work.

The change to the React in this PR vs PBNTR-479 mirrors the Rails version of the code much more closely - I realized that there was no need to pass the marginBottom prop from the Typeahead.tsx component to it's Control.tsx subcomponent because the marginBottom="none" in the subcomponent was being applied to a TextInput, not to anything Typeahead related (so no need to pass props).

Steps for PR completion:

  • Step 1: rectify this specific issue by reworking the React Typeahead margin bottom change (still inspired by the previous datepicker change) and alpha test immediately in nitro-web in this specific location.
  • Step 2: add in the rest of the work (Rails Typeahead margin bottom work and dark mode margin bottom work from PBNTR-479's PR into this one, and re-alpha test. (Note, the React Radio Children doc example alignment task will no longer included here as PBNTR-373 required a revert as well - this task will likely be a part of that ticket's fix PR).

Screenshots: Screenshots to visualize your addition/change
Rails margin bottom doc example
for PR rails dark mode mb
React margin bottom doc example
for PR react light mode mb
Datepicker dark mode margin bottom doc example works (compare vs current prod)
for PR datepicker rails dark

How to test? Steps to confirm the desired behavior:

  1. Go to margin bottom doc example (rails/react).
  2. Inspect each typeahead to see margin_bottom prop in action.
  3. Test typeaheads/typical paths in nitro-web milano environment to ensure works. The particular typeahead with issues initially inspiring the revert is located here (definitely test this one).

Checklist:

  • LABELS Add a label: enhancement, bug, improvement, new kit, deprecated, or breaking. See Changelog & Labels for details.
  • DEPLOY I have added the milano label to show I'm ready for a review.
  • TESTS I have added test coverage to my code.

@ElisaShapiro ElisaShapiro added enhancement New Features, Props, & Variants (USED IN CHANGELOG) milano 20 MAX - Deploy this PR to a review environment via Milano alpha labels Sep 10, 2024
@ElisaShapiro ElisaShapiro self-assigned this Sep 10, 2024
Copy link

🎉 Congratulations on creating an Alpha Version!

Your Alpha for Ruby Gems is 14.3.1.pre.alpha.PBNTR515typeaheadmarginbottomredux3746

Your Alpha for NPM is 14.3.1-alpha.PBNTR515typeaheadmarginbottomredux3746

Copy link

🎉 Congratulations on creating an Alpha Version!

Your Alpha for Ruby Gems is 14.3.1.pre.alpha.PBNTR515typeaheadmarginbottomredux3747

Your Alpha for NPM is 14.3.1-alpha.PBNTR515typeaheadmarginbottomredux3747

Copy link

🎉 Congratulations on creating an Alpha Version!

Your Alpha for Ruby Gems is 14.3.1.pre.alpha.PBNTR515typeaheadmarginbottomredux3751

Your Alpha for NPM is 14.3.1-alpha.PBNTR515typeaheadmarginbottomredux3751

Copy link

🎉 Congratulations on creating an Alpha Version!

Your Alpha for Ruby Gems is 14.3.1.pre.alpha.PBNTR515typeaheadmarginbottomredux3752

Your Alpha for NPM is 14.3.1-alpha.PBNTR515typeaheadmarginbottomredux3752

Copy link

🎉 Congratulations on creating an Alpha Version!

Your Alpha for Ruby Gems is 14.3.1.pre.alpha.PBNTR515typeaheadmarginbottomredux3750

Your Alpha for NPM is 14.3.1-alpha.PBNTR515typeaheadmarginbottomredux3750

Copy link

🎉 Congratulations on creating an Alpha Version!

Your Alpha for Ruby Gems is 14.3.2.pre.alpha.PBNTR515typeaheadmarginbottomredux3756

Your Alpha for NPM is 14.3.2-alpha.PBNTR515typeaheadmarginbottomredux3756

@ElisaShapiro ElisaShapiro marked this pull request as ready for review September 11, 2024 19:14
@ElisaShapiro ElisaShapiro requested a review from a team as a code owner September 11, 2024 19:14
@ElisaShapiro ElisaShapiro requested a review from a team as a code owner September 11, 2024 19:14
nidaqg
nidaqg previously approved these changes Sep 16, 2024
@nidaqg nidaqg added the Code Approved Approved by a Playbook Admin label Sep 16, 2024
Copy link

🎉 Congratulations on creating an Alpha Version!

Your Alpha for Ruby Gems is 14.3.2.pre.alpha.PBNTR515typeaheadmarginbottomredux3785

Your Alpha for NPM is 14.3.2-alpha.PBNTR515typeaheadmarginbottomredux3785

nidaqg
nidaqg previously approved these changes Sep 17, 2024
@nidaqg nidaqg added Product Approved pending technical review, OK to merge to master and removed Product Approved pending technical review, OK to merge to master labels Sep 17, 2024
@nidaqg nidaqg added this pull request to the merge queue Sep 20, 2024
Merged via the queue into master with commit 80f9134 Sep 20, 2024
5 checks passed
@nidaqg nidaqg deleted the PBNTR-515-typeahead-margin-bottom-redux branch September 20, 2024 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alpha Code Approved Approved by a Playbook Admin enhancement New Features, Props, & Variants (USED IN CHANGELOG) milano 20 MAX - Deploy this PR to a review environment via Milano Product Approved pending technical review, OK to merge to master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants