Skip to content

Commit

Permalink
[PBNTR-479] Expose margin bottom on Text Input within Typeahead (#3654)
Browse files Browse the repository at this point in the history
**What does this PR do?** A clear and concise description with your
runway ticket url.
[PBNTR-479](https://runway.powerhrg.com/backlog_items/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 showcase `margin_bottom` values 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](#3280), 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](https://runway.powerhrg.com/backlog_items/PBNTR-479)'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
<img width="1327" alt="for PR rails doc ex"
src="https://github.com/user-attachments/assets/9f285360-2140-4c2b-adb5-25efa9311b5f">
React margin bottom doc example
<img width="1324" alt="for PR react dark mode typeahead doc ex"
src="https://github.com/user-attachments/assets/178e8f92-f36f-4088-a383-16b0cfb94b43">

Incidental dark mode issue shown with datepicker:
margin bottom not working as expected in prod (see first three examples
all with margin_bottom small)
<img width="1319" alt="dark mode margin bottom wrong datepicker prod"
src="https://github.com/user-attachments/assets/e4a42e69-a4b5-4b14-b3e4-0b02ea65e9a5">
margin bottom fixed (all margin bottoms correctly responsive)
<img width="1479" alt="dark mode fix datepicker kit"
src="https://github.com/user-attachments/assets/6e09cc02-b54a-4f58-9a87-a5e776602811">


**How to test?** Steps to confirm the desired behavior:
1. Go to margin bottom doc example
([rails](https://pr3654.playbook.beta.px.powerapp.cloud/kits/typeahead#margin-bottom)/[react](https://pr3654.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://pr42318.nitro-web.beta.gm.powerapp.cloud/) to
ensure works.


#### 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.
  • Loading branch information
ElisaShapiro authored Sep 6, 2024
1 parent 4eeda54 commit cff399f
Show file tree
Hide file tree
Showing 12 changed files with 208 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,29 +18,32 @@ const RadioChildren = (props) => {
<div>
<Radio
label="Select"
marginBottom="sm"
name="Group1"
tabIndex={0}
value="Select"
{...props}
>
<Select
marginBottom="none"
minWidth="xs"
options={options}
/>
</Radio>
<Radio
label="Typeahead"
marginBottom="sm"
name="Group1"
tabIndex={0}
value="Typeahead"
{...props}
>
<Typeahead
marginBottom="none"
minWidth="xs"
options={options}
/>
</Radio>
<br />
<Radio
defaultChecked={false}
label="Typography"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
}
}
.text_input_wrapper {
margin-bottom: 1rem;
input::placeholder,
.text_input .placeholder {
@include pb_body_light_dark;
Expand Down
13 changes: 13 additions & 0 deletions playbook/app/pb_kits/playbook/pb_typeahead/_typeahead.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,17 @@ test('should pass className prop', () => {

const kit = screen.getByTestId('typeahead-test')
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')
const textInput = kit.querySelector(".pb_text_input_kit")
expect(textInput).toHaveClass("mb_sm")
})
9 changes: 8 additions & 1 deletion playbook/app/pb_kits/playbook/pb_typeahead/_typeahead.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ type TypeaheadProps = {
getOptionLabel?: string | (() => any),
getOptionValue?: string | (() => any),
name?: string,
marginBottom?: "none" | "xxs" | "xs" | "sm" | "md" | "lg" | "xl",
} & GlobalProps

export type SelectValueType = {
Expand Down Expand Up @@ -76,12 +77,18 @@ const Typeahead = ({
htmlOptions = {},
id,
loadOptions = noop,
marginBottom = "sm",
...props
}: TypeaheadProps) => {
const selectProps = {
cacheOptions: true,
components: {
Control,
Control: (controlProps: any) => (
<Control
{...controlProps}
marginBottom={marginBottom}
/>
),
ClearIndicator,
IndicatorsContainer,
IndicatorSeparator: null as null,
Expand Down
39 changes: 23 additions & 16 deletions playbook/app/pb_kits/playbook/pb_typeahead/components/Control.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,30 @@ import TextInput from '../../pb_text_input/_text_input'

type Props = {
selectProps: any,
marginBottom?: string,
}

const TypeaheadControl = (props: Props) => (
<div className="pb_typeahead_wrapper">
<TextInput
dark={props.selectProps.dark}
error={props.selectProps.error}
label={props.selectProps.label}
>
<Flex>
<components.Control
className="text_input"
{...props}
/>
</Flex>
</TextInput>
</div>
)
const TypeaheadControl = (props: Props) => {
const { selectProps, marginBottom } = props
const { dark, error, label } = selectProps

return (
<div className="pb_typeahead_wrapper">
<TextInput
dark={dark}
error={error}
label={label}
marginBottom={marginBottom}
>
<Flex>
<components.Control
className="text_input"
{...props}
/>
</Flex>
</TextInput>
</div>
)
}

export default TypeaheadControl
Original file line number Diff line number Diff line change
@@ -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 %>
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions playbook/app/pb_kits/playbook/pb_typeahead/docs/example.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ examples:
- typeahead_inline: Inline
- typeahead_multi_kit: Multi Kit Options
- typeahead_error_state: Error State
- typeahead_margin_bottom: Margin Bottom

react:
- typeahead_default: Default
Expand All @@ -23,3 +24,4 @@ examples:
- typeahead_async_createable: Createable (+ Async Data)
- typeahead_error_state: Error State
- typeahead_custom_menu_list: Custom MenuList
- typeahead_margin_bottom: Margin Bottom
1 change: 1 addition & 0 deletions playbook/app/pb_kits/playbook/pb_typeahead/docs/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ 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'
Original file line number Diff line number Diff line change
Expand Up @@ -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 %>
Expand Down
7 changes: 6 additions & 1 deletion playbook/app/pb_kits/playbook/pb_typeahead/typeahead.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ class Typeahead < Playbook::KitBase
default: false
prop :label
prop :load_options
prop :margin_bottom, type: Playbook::Props::Enum,
values: %w[none xxs xs sm md lg xl],
default: "sm"
prop :multi_kit, type: Playbook::Props::String,
default: ""
prop :name
Expand All @@ -36,7 +39,8 @@ class Typeahead < Playbook::KitBase
prop :value

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
Expand Down Expand Up @@ -65,6 +69,7 @@ def typeahead_react_options
inline: inline,
isMulti: is_multi,
label: label,
marginBottom: margin_bottom,
multiKit: multi_kit,
name: name,
options: options,
Expand Down
2 changes: 1 addition & 1 deletion playbook/spec/pb_kits/playbook/kits/typeahead_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,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

0 comments on commit cff399f

Please sign in to comment.