Skip to content

Commit

Permalink
[PBNTR-595] Follow up to Fixed Width Filter issue on Templates (#3827)
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-595](https://runway.powerhrg.com/backlog_items/PBNTR-595)
implements the popoverProps component on the Filter kit which will
unblock PBNTR's template work by allowing the team to set an optimal
Filter Popover width on our templates without using inline styling or a
separate scss file.

PopoverProps are connected to the embedded Popover within the Filter kit
(by spreading the popoverProps prop React, and by merging popover_props
to the rest of the props in Rails). A "Popover Props" doc example has
been added to the Rails and React kit pages which shows using the
popover prop to set a custom width on the filter popover. The Popover
kit itself now accepts a width prop to allow us to achieve this - no doc
example necessary at this time.

This is associated with an [alpha PR here](linktocome) to demonstrate
its use to prevent the Rails template popover width from jumping every
time an item is added to the Filter's Typeahead.

**Screenshots:** Screenshots to visualize your addition/change
Rails Popover Props doc example
<img width="1343" alt="for PR popover props doc ex rails"
src="https://github.com/user-attachments/assets/042ee607-5353-43c5-a583-aca143663577">
React Popover Props doc example
<img width="1338" alt="for PR popover props doc ex react"
src="https://github.com/user-attachments/assets/bb2d7d15-f52b-4772-9aee-ece6906bc69e">
Rails Template with `popover_props` applying a set width to avoid the
filter popover jumping with each additional typeahead entry
desktop+mobile
<img width="1530" alt="for PR rails template desktop filter fixed width"
src="https://github.com/user-attachments/assets/a9545f90-af36-4f2a-9dcb-b9a3c25f44b2">
<img width="194" alt="for PR rails template mobile filter fixed width"
src="https://github.com/user-attachments/assets/6e658743-4146-41e9-8e80-74f9c7494452">


**How to test?** Steps to confirm the desired behavior:
1. Go to the filter kit review env page, specifically to the Popover
Props doc example
([rails](https://pr3827.playbook.beta.px.powerapp.cloud/kits/filter#popover-props),
[react](https://pr3827.playbook.beta.px.powerapp.cloud/kits/filter/react#popover-props))
2. Click on Filter popover button and see a smaller-than-usual Filter
popover appear. Compare to other doc examples on the page (default,
etc.) to see the difference.
3. Go to the [rails template
page](https://pr43315.nitro-web.beta.hq.powerapp.cloud/dev_docs/playbook/templates/demos/basic_table/rails)
in the review environment with alpha from this PR.
4. Open the Filter and add several items to the typeahead. The width of
the popover should not jump with every additional entry.


#### 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.
~~- [ ] **TESTS** I have added test coverage to my code.~~
  • Loading branch information
ElisaShapiro authored Oct 22, 2024
1 parent d7b8e5e commit a04ad24
Show file tree
Hide file tree
Showing 14 changed files with 139 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const FilterDouble = ({
maxHeight,
minWidth,
placement,
popoverProps,
...bgProps
}: FilterDoubleProps): React.ReactElement => (
<FilterBackground
Expand All @@ -49,6 +50,7 @@ const FilterDouble = ({
maxHeight={maxHeight}
minWidth={minWidth}
placement={placement}
popoverProps={popoverProps}
>
{children}
</FiltersPopover>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const FilterSingle = ({
maxHeight,
minWidth,
placement,
popoverProps,
...bgProps
}: FilterSingleProps): React.ReactElement => {
return (
Expand All @@ -52,6 +53,7 @@ const FilterSingle = ({
maxHeight={maxHeight}
minWidth={minWidth}
placement={placement}
popoverProps={popoverProps}
>
{children}
</FiltersPopover>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,17 @@ import React, { ReactNode, useState } from 'react'

import CircleIconButton from '../../pb_circle_icon_button/_circle_icon_button'
import PbReactPopover from '../../pb_popover/_popover'
import { GenericObject } from '../../types'

type FiltersPopoverProps = {
children?: React.ReactChild[] | React.ReactChild | (({closePopover}: {closePopover: () => void}) => ReactNode),
dark?: boolean,
maxHeight?: string,
minWidth?: string,
placement?: "top" | "right" | "bottom" | "left" | "top-start" | "top-end" | "bottom-start" | "bottom-end" | "right-start" | "right-end" | "left-start" | "left-end",
popoverProps?: GenericObject,
}
const FiltersPopover = ({ children, dark, maxHeight, minWidth, placement = "bottom-start" }: FiltersPopoverProps): React.ReactElement => {
const FiltersPopover = ({ children, dark, maxHeight, minWidth, placement = "bottom-start", popoverProps }: FiltersPopoverProps): React.ReactElement => {
const [hide, updateHide] = useState(true)
const toggle = () => updateHide(!hide)

Expand All @@ -33,6 +35,7 @@ const FiltersPopover = ({ children, dark, maxHeight, minWidth, placement = "bott
reference={filterButton}
shouldClosePopover={updateHide}
show={!hide}
{...popoverProps}
>
<div className="pb-form">
{typeof children === 'function'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<%=
pb_rails("filter", props: {
id: "filter_popover_props",
position: "top",
filters: [
{ name: "name", value: "John Wick" },
{ name: "city", value: "San Francisco"}
],
sort_menu: [
{ item: "Popularity", link: "?q[sorts]=managers_popularity+asc", active: true, direction: "desc" },
{ item: "Mananger's Title", link: "?q[sorts]=managers_title+asc", active: false },
{ item: "Manager's Name", link: "?q[sorts]=managers_name+asc", active: false },
],
template: "default",
results: 1,
popover_props: { width: "250px" },
}) do
%>
<%
example_collection = [
OpenStruct.new(name: "USA", value: 1),
OpenStruct.new(name: "Canada", value: 2),
OpenStruct.new(name: "Brazil", value: 3),
OpenStruct.new(name: "Philippines", value: 4),
OpenStruct.new(name: "A galaxy far far away, like really far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far away...", value: 5)
]
%>
<%= pb_rails("form", props: { form_system_options: { scope: :example, method: :get } }) do |form| %>
<%= form.text_field :example_text_field, props: { label: true } %>
<%= form.collection_select :example_collection_select, example_collection, :value, :name, props: {max_width: "sm", label: true } %>

<%= form.actions do |action| %>
<%= action.submit props: {
text: "Apply",
data: {
disable_with: "pb_rails('icon', props: { icon: 'spinner', spin: true, fixed_width: true })Searching...".html_safe
},}%>
<%= action.button props: { type: "reset", text: "Clear", variant: "secondary" } %>
<% end %>
<% end %>
<% end %>
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import React from 'react'
import { Button, Filter, Flex, Select, TextInput } from 'playbook-ui'

const FilterPopoverProps = (props) => {
const options = [
{ value: 'USA' },
{ value: 'Canada' },
{ value: 'Brazil' },
{ value: 'Philippines' },
{ value: 'A galaxy far far away, like really far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far far away...' },
]

const popoverProps = {
width: "250px"
}

return (
<Filter
{...props}
double
filters={{
'Full Name': 'John Wick',
'City': 'San Francisco',
}}
popoverProps={popoverProps}
results={1}
sortOptions={{
popularity: 'Popularity',
// eslint-disable-next-line
manager_title: 'Manager\'s Title',
// eslint-disable-next-line
manager_name: 'Manager\'s Name',
}}
sortValue={[{ name: 'popularity', dir: 'desc' }]}
>
{({ closePopover }) => (
<form>
<TextInput
label="Example Text Field"
placeholder="Enter Text"
{...props}
/>
<Select
blankSelection="Select One..."
label="Example Collection Select"
name="Collection Select"
options={options}
{...props}
/>
<Flex
spacing="between"
{...props}
>
<Button
onClick={closePopover}
text="Apply"
{...props}
/>
<Button
text="Clear"
variant="secondary"
{...props}
/>
</Flex>
</form>
)}
</Filter>
)
}

export default FilterPopoverProps
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This kit uses the [Popover kit](https://playbook.powerapp.cloud/kits/popover) under the hood for the Filter Popover which comes with its own set of props. If you want to apply certain Popover props to that underlying kit, you can do so by using the optional `popover_props` prop. This prop must be an object that contains valid Popover props. For a full list of Popover props, see [here](https://playbook.powerapp.cloud/kits/popover).
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This kit uses the [Popover kit](https://playbook.powerapp.cloud/kits/popover/react) under the hood for the Filter Popover which comes with its own set of props. If you want to apply certain Popover props to that underlying kit, you can do so by using the optional `popoverProps` prop. This prop must be an object that contains valid Popover props. For a full list of Popover props, see [here](https://playbook.powerapp.cloud/kits/popover/react).
3 changes: 3 additions & 0 deletions playbook/app/pb_kits/playbook/pb_filter/docs/example.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ examples:
- filter_max_width: Max Width for Popover Inside of Filter
- filter_max_height: Max Height for Popover Inside of Filter
- filter_placement: Filter Placement
- filter_popover_props: Popover Props

react:
- filter_default: Default
Expand All @@ -21,3 +22,5 @@ examples:
- filter_max_width: Max Width for Popover Inside of Filter
- filter_max_height: Max Height for Popover Inside of Filter
- filter_placement: Filter Placement
- filter_popover_props: Popover Props

1 change: 1 addition & 0 deletions playbook/app/pb_kits/playbook/pb_filter/docs/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ export { default as SortOnly } from './_sort_only.jsx'
export { default as FilterMaxWidth } from './_filter_max_width.jsx'
export { default as FilterMaxHeight } from './_filter_max_height.jsx'
export { default as FilterPlacement } from './_filter_placement.jsx'
export { default as FilterPopoverProps } from './_filter_popover_props.jsx'
4 changes: 2 additions & 2 deletions playbook/app/pb_kits/playbook/pb_filter/filter.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,13 @@
<% end %>

<% if object.template != "sort_only"%>
<%= pb_rails("popover", props: {max_height: object.max_height, min_width: object.min_width, close_on_click: "outside", trigger_element_id: "filter#{object.id}", tooltip_id: "filter-form#{object.id}", position: object.placement }) do %>
<%= pb_rails("popover", props: {max_height: object.max_height, min_width: object.min_width, close_on_click: "outside", trigger_element_id: "filter#{object.id}", tooltip_id: "filter-form#{object.id}", position: object.placement }.merge(object.popover_props)) do %>
<%= content %>
<% end %>
<%end%>

<% if object.template != "filter_only"%>
<%= pb_rails("popover", props: {max_height: object.max_height, classname: "pb_filter_sort_menu", close_on_click: "outside", trigger_element_id: "sort-button#{object.id}", tooltip_id: "sort-filter-btn-tooltip#{object.id}", position: object.placement , padding: 'none'}) do %>
<%= pb_rails("popover", props: {max_height: object.max_height, classname: "pb_filter_sort_menu", close_on_click: "outside", trigger_element_id: "sort-button#{object.id}", tooltip_id: "sort-filter-btn-tooltip#{object.id}", position: object.placement , padding: 'none'}.merge(object.popover_props)) do %>
<%= pb_rails("list") do %>
<% object.sort_menu.each do |item| %>
<%= pb_rails("list/item") do%> <%= pb_rails("button", props: {variant: "link" ,classname: "p-0", text: item[:item], link: item[:link]}) %><% end %>
Expand Down
2 changes: 2 additions & 0 deletions playbook/app/pb_kits/playbook/pb_filter/filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ class Filter < Playbook::KitBase
prop :placement, type: Playbook::Props::Enum,
values: %w[top bottom left right top-start top-end bottom-start bottom-end right-start right-end left-start left-end],
default: "bottom-start"
prop :popover_props, type: Playbook::Props::HashProp,
default: {}

def classname
generate_classname("pb_filter_kit").rstrip
Expand Down
6 changes: 5 additions & 1 deletion playbook/app/pb_kits/playbook/pb_popover/_popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ const Popover = (props: PbPopoverProps) => {
maxWidth,
minHeight,
minWidth,
width,
targetId,
} = props;

Expand All @@ -97,7 +98,8 @@ const Popover = (props: PbPopoverProps) => {
maxHeight ? { maxHeight: maxHeight } : {},
maxWidth ? { maxWidth: maxWidth } : {},
minHeight ? { minHeight: minHeight } : {},
minWidth ? { minWidth: minWidth } : {}
minWidth ? { minWidth: minWidth } : {},
width ? { width: width } : {}
);
};
const ariaProps = buildAriaProps(aria);
Expand Down Expand Up @@ -167,6 +169,7 @@ const PbReactPopover = (props: PbPopoverProps): React.ReactElement => {
maxWidth,
minHeight,
minWidth,
width,
} = props;

useEffect(() => {
Expand Down Expand Up @@ -216,6 +219,7 @@ const PbReactPopover = (props: PbPopoverProps): React.ReactElement => {
placement={placement}
referenceElement={referenceElement}
targetId={targetId}
width={width}
zIndex={zIndex}
{...props}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
trigger_element_id: "regular-popover-1",
tooltip_id: "tooltip-1",
offset: true,
position: "top"
position: "top",
}) do %>
I'm a popover. I can show content of any size.
<% end %>
Expand Down
4 changes: 3 additions & 1 deletion playbook/app/pb_kits/playbook/pb_popover/popover.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class Popover < Playbook::KitBase
prop :max_width
prop :min_width
prop :min_height
prop :width
prop :z_index, type: Playbook::Props::String
prop :offset, type: Playbook::Props::Boolean, default: false
prop :close_on_click, type: Playbook::Props::Enum,
Expand All @@ -36,11 +37,12 @@ def width_height_helper
out += "max-width: #{max_width}; " if max_width.present?
out += "min-height: #{min_height}; " if min_height.present?
out += "min-width: #{min_width};" if min_width.present?
out += "width: #{width};" if width.present?
out
end

def width_height_class_helper
"overflow_handling" if max_height || max_width
"overflow_handling" if max_height || max_width || width
end

def data
Expand Down

0 comments on commit a04ad24

Please sign in to comment.