[PBNTR-479] Expose margin bottom on Text Input within Typeahead #3654
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ElisaShapiro
added
the
enhancement
ElisaShapiro
added
milano
|
🎉 Congratulations on creating an Alpha Version! Your Alpha for Ruby Gems is 14.2.1.pre.alpha.PBNTR479removeextraspacingfromradiochildren3669 Your Alpha for NPM is 14.2.1-alpha.PBNTR479removeextraspacingfromradiochildren3669 |
|
🎉 Congratulations on creating an Alpha Version! Your Alpha for Ruby Gems is 14.2.1.pre.alpha.PBNTR479removeextraspacingfromradiochildren3671 Your Alpha for NPM is 14.2.1-alpha.PBNTR479removeextraspacingfromradiochildren3671 |
nidaqg
previously approved these changes
Sep 6, 2024
3 tasks
nidaqg
approved these changes
Sep 6, 2024
nidaqg
deleted the
PBNTR-479-remove-extra-spacing-from-radio-children
branch
5 tasks
github-merge-queue bot
pushed a commit
that referenced
this pull request
Sep 20, 2024
…479 (#3680) **What does this PR do?** A clear and concise description with your runway ticket url. [PBNTR-515](https://runway.powerhrg.com/backlog_items/PBNTR-515) is attempt #2 at [PBNTR-479](https://runway.powerhrg.com/backlog_items/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](https://playbook.powerapp.cloud/kits/typeahead/react#createable-async-data) typeahead used here ([nitroqa link](https://nitro.powerhrg.com/connect/nitroqa.powerhrg.com/powerlife/admin/dizzles_sizzle/new), [component code link](https://github.com/powerhome/nitro-web/blob/297ab6ada0a694f65d91740d17572c2b04d98036/components/brand_headlines/app/javascript/Shared/TagSelector.tsx#L5)) 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: - [x] 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. - [x] 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](#3654) into this one, and re-alpha test. (Note, the [React Radio Children doc example](https://playbook.powerapp.cloud/kits/radio/react#children) 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 <img width="1308" alt="for PR rails dark mode mb" src="https://github.com/user-attachments/assets/03cab2f3-6c4a-4713-ac4f-3b3d40e08c32"> React margin bottom doc example <img width="1319" alt="for PR react light mode mb" src="https://github.com/user-attachments/assets/3d0c4a0c-0ae9-4826-8e1e-c536544a1765"> Datepicker dark mode margin bottom doc example works (compare vs current prod) <img width="1316" alt="for PR datepicker rails dark" src="https://github.com/user-attachments/assets/761c0100-4335-4bb7-89ae-18ebfde9e548"> **How to test?** Steps to confirm the desired behavior: 1. Go to margin bottom doc example ([rails](https://pr3680.playbook.beta.px.powerapp.cloud/kits/typeahead#margin-bottom)/[react](https://pr3690.playbook.beta.px.powerapp.cloud/kits/typeahead/react#margin-bottom)). 2. Inspect each typeahead to see margin_bottom prop in action. 3. Test typeaheads/typical paths in [nitro-web milano environment](https://pr42423.nitro-web.beta.gm.powerapp.cloud/) to ensure works. The particular typeahead with issues initially inspiring the revert is located [here](https://pr42423.nitro-web.beta.gm.powerapp.cloud/powerlife/admin/dizzles_sizzle/new) (definitely test this one). #### Checklist: - [x] **LABELS** Add a label: `enhancement`, `bug`, `improvement`, `new kit`, `deprecated`, or `breaking`. See [Changelog & Labels](https://github.com/powerhome/playbook/wiki/Changelog-&-Labels) for details. - [x] **DEPLOY** I have added the `milano` label to show I'm ready for a review. - [x] **TESTS** I have added test coverage to my code.
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do? A clear and concise description with your runway ticket url.
PBNTR-479 exposes the margin bottom on the text input that lies within a wrapper inside the Typeahead kit so that a dev can to modify kit's
margin_bottom. The doc examples showcasemargin_bottomvalues of: none, xxs, xs, sm, md, lg, and xl. The Rails doc example show a react-rendered rails version of the kit but functions the same in the rails-only version as well. The initial doc example showed both rails versions within the doc example but that felt like overkill.This work is modeled after work previously done on the datepicker kit here, and functions the same. There was an additional incidental finding discovered here: the dark margin bottom doc examples for both typeahead (rails and react) and datepicker (rails) did not initially work as expected due to the margin_bottom still being hardcoded into the dark mode section of
text_input.scss. All doc examples where the prescribed margin_bottom was small or smaller (none through sm) had margin_bottom values that calculated to sm due to the hardcoded value on the text-input-wrapper. The larger ones visually appeared right but upon inspection one can see the additional margin on the text-input-wrapper. This is fixed in this PR (see datepicker screenshots in screenshot section).PBNTR-373's doc examples will require the work from this ticket as this previously-unreachable internal margin prevents the ability to vertically align the child Typeahead kit with the Radio button.
Screenshots: Screenshots to visualize your addition/change
Rails margin bottom doc example
React margin bottom doc example
Incidental dark mode issue shown with datepicker:
margin bottom not working as expected in prod (see first three examples all with margin_bottom small)
margin bottom fixed (all margin bottoms correctly responsive)
How to test? Steps to confirm the desired behavior:
Checklist:
enhancement,bug,improvement,new kit,deprecated, orbreaking. See Changelog & Labels for details.milanolabel to show I'm ready for a review.