[PBNTR-515] Remove margin bottom from Typeahead kit, un-revert PBNTR-…

…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.
powerhome · Sep 20, 2024 · 80f9134 · 80f9134
1 parent f7117bf
commit 80f9134
Show file tree

Hide file tree

Showing 13 changed files with 185 additions and 10 deletions.
diff --git a/playbook/app/pb_kits/playbook/pb_text_input/_text_input.scss b/playbook/app/pb_kits/playbook/pb_text_input/_text_input.scss
@@ -45,7 +45,6 @@
       }
     }
     .text_input_wrapper {
-      margin-bottom: 1rem;
       input::placeholder,
       .text_input .placeholder {
         @include pb_body_light_dark;

diff --git a/playbook/app/pb_kits/playbook/pb_typeahead/_typeahead.scss b/playbook/app/pb_kits/playbook/pb_typeahead/_typeahead.scss
@@ -68,7 +68,6 @@
     opacity: 0;
   }
   [class^=pb_list_kit] {
-    margin-top: -$space-sm;
     li {
       transition: background-color .25s ease-in-out;
     }

diff --git a/playbook/app/pb_kits/playbook/pb_typeahead/_typeahead.test.jsx b/playbook/app/pb_kits/playbook/pb_typeahead/_typeahead.test.jsx
@@ -109,6 +109,20 @@ test('should pass className prop', () => {
   expect(kit).toHaveClass(className)
 })
 
+test('typeahead textinput has mb_sm class by default', () => {
+  render(
+    <Typeahead
+        data={{ testid: 'default-mb-test' }}
+        options={options} 
+    />
+  )
+
+  const kit = screen.getByTestId('default-mb-test')
+  expect(kit).toHaveClass("pb_typeahead_kit mb_sm")
+  const textInput = kit.querySelector(".pb_text_input_kit")
+  expect(textInput).toHaveClass("mb_none")
+})
+
 test('typeahead with colored pills', () => {
   render(
     <Typeahead

diff --git a/playbook/app/pb_kits/playbook/pb_typeahead/_typeahead.tsx b/playbook/app/pb_kits/playbook/pb_typeahead/_typeahead.tsx
@@ -45,6 +45,7 @@ type TypeaheadProps = {
   getOptionLabel?: string | (() => any),
   getOptionValue?: string | (() => any),
   name?: string,
+  marginBottom?: "none" | "xxs" | "xs" | "sm" | "md" | "lg" | "xl",
   pillColor?: "primary" | "neutral" | "success" | "warning" | "error" | "info" | "data_1" | "data_2" | "data_3" | "data_4" | "data_5" | "data_6" | "data_7" | "data_8" | "windows" | "siding" | "roofing" | "doors" | "gutters" | "solar" | "insulation" | "accessories",
 } & GlobalProps
 
@@ -78,6 +79,7 @@ const Typeahead = ({
   htmlOptions = {},
   id,
   loadOptions = noop,
+  marginBottom = "sm",
   pillColor,
   ...props
 }: TypeaheadProps) => {
@@ -138,6 +140,7 @@ const Typeahead = ({
   const htmlProps = buildHtmlProps(htmlOptions)
   const classes = classnames(
     'pb_typeahead_kit react-select',
+    `mb_${marginBottom}`,
     globalProps(props),
     className
   )

diff --git a/playbook/app/pb_kits/playbook/pb_typeahead/components/Control.tsx b/playbook/app/pb_kits/playbook/pb_typeahead/components/Control.tsx
@@ -11,14 +11,15 @@ type Props = {
 const TypeaheadControl = (props: Props) => (
   <div className="pb_typeahead_wrapper">
     <TextInput
-      dark={props.selectProps.dark}
-      error={props.selectProps.error}
-      label={props.selectProps.label}
+        dark={props.selectProps.dark}
+        error={props.selectProps.error}
+        label={props.selectProps.label}
+        marginBottom="none"
     >
       <Flex>
         <components.Control
-          className="text_input"
-          {...props}
+            className="text_input"
+            {...props}
         />
       </Flex>
     </TextInput>

diff --git a/playbook/app/pb_kits/playbook/pb_typeahead/docs/_typeahead_margin_bottom.html.erb b/playbook/app/pb_kits/playbook/pb_typeahead/docs/_typeahead_margin_bottom.html.erb
@@ -0,0 +1,88 @@
+<%
+  options = [
+    { label: 'Orange', value: '#FFA500' },
+    { label: 'Red', value: '#FF0000' },
+    { label: 'Green', value: '#00FF00' },
+    { label: 'Blue', value: '#0000FF' },
+  ]
+%>
+
+<%= pb_rails("typeahead", props: {
+    id: "typeahead-default",
+    placeholder: "All Colors",
+    options: options,
+    label: "None",
+    name: :foo,
+    is_multi: false,
+    margin_bottom: "none",
+  })
+%>
+<%= pb_rails("typeahead", props: {
+    id: "typeahead-default",
+    placeholder: "All Colors",
+    options: options,
+    label: "XXS",
+    name: :foo,
+    is_multi: false,
+    margin_bottom: "xxs",
+  })
+%>
+<%= pb_rails("typeahead", props: {
+    id: "typeahead-default",
+    placeholder: "All Colors",
+    options: options,
+    label: "XS",
+    name: :foo,
+    is_multi: false,
+    margin_bottom: "xs",
+  })
+%>
+<%= pb_rails("typeahead", props: {
+    id: "typeahead-default",
+    placeholder: "All Colors",
+    options: options,
+    label: "Default - SM",
+    name: :foo,
+    is_multi: false,
+  })
+%>
+<%= pb_rails("typeahead", props: {
+    id: "typeahead-default",
+    placeholder: "All Colors",
+    options: options,
+    label: "MD",
+    name: :foo,
+    is_multi: false,
+    margin_bottom: "md",
+  })
+%>
+<%= pb_rails("typeahead", props: {
+    id: "typeahead-default",
+    placeholder: "All Colors",
+    options: options,
+    label: "LG",
+    name: :foo,
+    is_multi: false,
+    margin_bottom: "lg",
+  })
+%>
+<%= pb_rails("typeahead", props: {
+    id: "typeahead-default",
+    placeholder: "All Colors",
+    options: options,
+    label: "XL",
+    name: :foo,
+    is_multi: false,
+    margin_bottom: "xl",
+  })
+%>
+
+<%= javascript_tag defer: "defer" do %>
+  document.addEventListener("pb-typeahead-kit-typeahead-default-result-option-select", function(event) {
+    console.log('Single Option selected')
+    console.dir(event.detail)
+  })
+  document.addEventListener("pb-typeahead-kit-typeahead-default-result-clear", function() {
+    console.log('All options cleared')
+  })
+<% end %>
diff --git a/playbook/app/pb_kits/playbook/pb_typeahead/docs/_typeahead_margin_bottom.jsx b/playbook/app/pb_kits/playbook/pb_typeahead/docs/_typeahead_margin_bottom.jsx
@@ -0,0 +1,60 @@
+import React from 'react'
+
+import Typeahead from '../_typeahead'
+
+const options = [
+  { label: 'Orange', value: '#FFA500' },
+  { label: 'Red', value: '#FF0000' },
+  { label: 'Green', value: '#00FF00' },
+  { label: 'Blue', value: '#0000FF' },
+]
+
+const TypeaheadMarginBottom = (props) => {
+  return (
+    <>
+      <Typeahead
+          label="None"
+          marginBottom="none"
+          options={options}
+          {...props}
+      />
+      <Typeahead
+          label="XXS"
+          marginBottom="xxs"
+          options={options}
+          {...props}
+      />   
+      <Typeahead
+          label="XS"
+          marginBottom="xs"
+          options={options}
+          {...props}
+      />   
+      <Typeahead
+          label="Default - SM"
+          options={options}
+          {...props}
+      />   
+      <Typeahead
+          label="MD"
+          marginBottom="md"
+          options={options}
+          {...props}
+      />
+      <Typeahead
+          label="LG"
+          marginBottom="lg"
+          options={options}
+          {...props}
+      />
+      <Typeahead
+          label="XL"
+          marginBottom="xl"
+          options={options}
+          {...props}
+      />
+    </>
+  )
+}
+
+export default TypeaheadMarginBottom
diff --git a/playbook/app/pb_kits/playbook/pb_typeahead/docs/example.yml b/playbook/app/pb_kits/playbook/pb_typeahead/docs/example.yml
@@ -9,6 +9,7 @@ examples:
     - typeahead_inline: Inline
     - typeahead_multi_kit: Multi Kit Options
     - typeahead_error_state: Error State
+    - typeahead_margin_bottom: Margin Bottom
     - typeahead_with_pills_color: With Pills (Custom Color)
 
   react:
@@ -24,4 +25,5 @@ examples:
     - typeahead_async_createable: Createable (+ Async Data)
     - typeahead_error_state: Error State
     - typeahead_custom_menu_list: Custom MenuList
+    - typeahead_margin_bottom: Margin Bottom
     - typeahead_with_pills_color: With Pills (Custom Color)
diff --git a/playbook/app/pb_kits/playbook/pb_typeahead/docs/index.js b/playbook/app/pb_kits/playbook/pb_typeahead/docs/index.js
@@ -10,4 +10,5 @@ export { default as TypeaheadCreateable } from './_typeahead_createable.jsx'
 export { default as TypeaheadAsyncCreateable } from './_typeahead_async_createable.jsx'
 export { default as TypeaheadErrorState } from './_typeahead_error_state.jsx'
 export { default as TypeaheadCustomMenuList } from './_typeahead_custom_menu_list.jsx'
+export { default as TypeaheadMarginBottom } from './_typeahead_margin_bottom.jsx'
 export { default as TypeaheadWithPillsColor } from './_typeahead_with_pills_color.jsx'
diff --git a/playbook/app/pb_kits/playbook/pb_typeahead/typeahead.html.erb b/playbook/app/pb_kits/playbook/pb_typeahead/typeahead.html.erb
@@ -20,7 +20,8 @@
         label: object.label,
         name: object.name,
         value: object.value,
-        placeholder: object.placeholder
+        placeholder: object.placeholder,
+        margin_bottom: "none",
       }) %>
       <%= pb_rails("list", props: { ordered: false, borderless: false, xpadding: true, role: "status", aria: { live: "polite" }, data: { pb_typeahead_kit_results: true } }) do %>
       <% end %>

diff --git a/playbook/app/pb_kits/playbook/pb_typeahead/typeahead.rb b/playbook/app/pb_kits/playbook/pb_typeahead/typeahead.rb
@@ -34,12 +34,16 @@ class Typeahead < Playbook::KitBase
       prop :search_term_minimum_length, default: 3
       prop :search_debounce_timeout, default: 250
       prop :value
+      prop :margin_bottom, type: Playbook::Props::Enum,
+                           values: %w[none xxs xs sm md lg xl],
+                           default: "sm"
       prop :pill_color, type: Playbook::Props::Enum,
                         values: %w[primary neutral success warning error info data_1 data_2 data_3 data_4 data_5 data_6 data_7 data_8 windows siding roofing doors gutters solar insulation accessories],
                         default: "primary"
 
       def classname
-        generate_classname("pb_typeahead_kit")
+        default_margin_bottom = margin_bottom.present? ? "" : " mb_sm"
+        generate_classname("pb_typeahead_kit") + default_margin_bottom
       end
 
       def inline_class
@@ -69,6 +73,7 @@ def typeahead_react_options
           inline: inline,
           isMulti: is_multi,
           label: label,
+          marginBottom: margin_bottom,
           multiKit: multi_kit,
           name: name,
           options: options,

diff --git a/playbook/spec/pb_kits/playbook/kits/typeahead_spec.rb b/playbook/spec/pb_kits/playbook/kits/typeahead_spec.rb
@@ -17,6 +17,7 @@
   it { is_expected.to define_prop(:inline).with_default(false) }
   it { is_expected.to define_prop(:label) }
   it { is_expected.to define_prop(:load_options) }
+  it { is_expected.to define_prop(:margin_bottom).with_default("sm") }
   it { is_expected.to define_prop(:name) }
   it {
     is_expected.to define_prop(:options)
@@ -32,7 +33,7 @@
 
   describe "#classname" do
     it "returns namespaced class name", :aggregate_failures do
-      expect(subject.new({}).classname).to eq "pb_typeahead_kit"
+      expect(subject.new({}).classname).to eq "pb_typeahead_kit mb_sm"
     end
   end
 end
diff --git a/playbook/spec/pb_kits/playbook/pb_typeahead/typeahead_spec.rb b/playbook/spec/pb_kits/playbook/pb_typeahead/typeahead_spec.rb
@@ -8,6 +8,7 @@
   it { is_expected.to define_prop(:async).with_default(false) }
   it { is_expected.to define_prop(:label) }
   it { is_expected.to define_prop(:load_options) }
+  it { is_expected.to define_prop(:margin_bottom).with_default("sm") }
   it { is_expected.to define_prop(:multi_kit).with_default("") }
   it { is_expected.to define_prop(:name) }
   it { is_expected.to define_prop(:options).with_default([]) }