Skip to content

Commit

Permalink
[IndexFilters] Add support for passing zIndexOverride to disclosures (#…
Browse files Browse the repository at this point in the history
…11883)

### WHY are these changes introduced?

`Filters`, `IndexFilters`, and `Tabs` all render `Popovers` and
`Tooltips`, which support a `zIndexOverride` when needed by consuming
apps. For example, the SettingsModal has a `z-index` of 516, which is
higher than `Popover` and `Tooltip`. When rendered in a /settings view,
these need to have an increased `z-index` from `--p-z-index-2` (400) to
`--p-z-index-9` (517).

### WHAT is this pull request doing?

This PR simply adds support for passing through a `z-index` override to
the `Popover` and `Tooltip` components composed by `Filters`,
`IndexFilters`, and `Tabs` through a new `disclosureZIndexOverride`
prop.

### How to 🎩

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#install-dependencies-and-build-workspaces)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

### 🎩 checklist

- [ ] Tested a
[snapshot](https://github.com/Shopify/polaris/blob/main/documentation/Releasing.md#-snapshot-releases)
(pending)
- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
  • Loading branch information
chloerice authored Apr 16, 2024
1 parent 3740304 commit a60d8aa
Show file tree
Hide file tree
Showing 13 changed files with 229 additions and 19 deletions.
5 changes: 5 additions & 0 deletions .changeset/all-humans-cry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris': minor
---

Added a `disclosureZIndexOverride` prop to `Filters`, `IndexFilters`, and `Tabs` that is passed to `Popover` and `Tooltip` when provided
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,16 @@ export interface FilterPillProps extends FilterInterface {
selected?: boolean;
/** Whether the Popover will be initially open or not */
initialActive: boolean;
/** Callback invoked when the filter is removed */
onRemove?(key: string): void;
/** Callback invoked when the filter is clicked */
onClick?(key: string): void;
/** Whether filtering is disabled */
disabled?: boolean;
/** Override z-index of popovers and tooltips */
disclosureZIndexOverride?: number;
/** Whether the filter should close when clicking inside another Popover. */
closeOnChildOverlayClick?: boolean;
/** Callback invoked when the filter is removed */
onRemove?(key: string): void;
/** Callback invoked when the filter is clicked */
onClick?(key: string): void;
}

export function FilterPill({
Expand All @@ -44,6 +46,7 @@ export function FilterPill({
hideClearButton,
selected,
initialActive,
disclosureZIndexOverride,
closeOnChildOverlayClick,
onRemove,
onClick,
Expand Down Expand Up @@ -207,6 +210,7 @@ export function FilterPill({
key={filterKey}
onClose={handlePopoverClose}
preferredAlignment="left"
zIndexOverride={disclosureZIndexOverride}
preventCloseOnChildOverlayClick={!closeOnChildOverlayClick}
>
<div className={styles.PopoverWrapper}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,21 @@ describe('<Filters />', () => {
});
});

it('passes the disclosureZIndexOverride to Popover when provided', () => {
const disclosureZIndexOverride = 517;
const wrapper = mountWithApp(
<FilterPill
{...defaultProps}
disclosureZIndexOverride={disclosureZIndexOverride}
/>,
);
const activator = wrapper.find(UnstyledButton);
activator?.trigger('onClick');
expect(wrapper).toContainReactComponent(Popover, {
zIndexOverride: disclosureZIndexOverride,
});
});

it('invokes the onRemove callback when clicking the clear button inside the Popover', () => {
const spy = jest.fn();
const wrapper = mountWithApp(
Expand Down
9 changes: 9 additions & 0 deletions polaris-react/src/components/IndexFilters/IndexFilters.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ export interface IndexFiltersProps
onEditStart?: (mode: ActionableIndexFiltersMode) => void;
/** The current mode of the IndexFilters component. Used to determine which view to show */
mode: IndexFiltersMode;
/** Override z-index of popovers and tooltips */
disclosureZIndexOverride?: number;
/** Callback to set the mode of the IndexFilters component */
setMode: (mode: IndexFiltersMode) => void;
/** Will disable all the elements within the IndexFilters component */
Expand Down Expand Up @@ -136,6 +138,7 @@ export function IndexFilters({
loading,
mode,
setMode,
disclosureZIndexOverride,
disableStickyMode,
isFlushWhenSticky = false,
canCreateNewView = true,
Expand Down Expand Up @@ -280,6 +283,7 @@ export function IndexFilters({
onChangeKey={onSortKeyChange}
onChangeDirection={onSortDirectionChange}
disabled={disabled}
disclosureZIndexOverride={disclosureZIndexOverride}
/>
);
}, [
Expand All @@ -289,6 +293,7 @@ export function IndexFilters({
sortOptions,
sortSelected,
disabled,
disclosureZIndexOverride,
]);

function handleClickEditColumnsButton() {
Expand Down Expand Up @@ -398,6 +403,7 @@ export function IndexFilters({
disabled={Boolean(
mode !== IndexFiltersMode.Default || disabled,
)}
disclosureZIndexOverride={disclosureZIndexOverride}
canCreateNewView={canCreateNewView}
onCreateNewView={onCreateNewView}
/>
Expand Down Expand Up @@ -428,6 +434,9 @@ export function IndexFilters({
...defaultStyle,
...transitionStyles[state],
}}
disclosureZIndexOverride={
disclosureZIndexOverride
}
/>
)}
{editColumnsMarkup}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export interface SearchFilterButtonProps {
label: string;
disabled?: boolean;
tooltipContent: string;
disclosureZIndexOverride?: number;
hideFilters?: boolean;
hideQueryField?: boolean;
style: CSSProperties;
Expand All @@ -23,6 +24,7 @@ export function SearchFilterButton({
label,
disabled,
tooltipContent,
disclosureZIndexOverride,
style,
hideFilters,
hideQueryField,
Expand Down Expand Up @@ -53,7 +55,12 @@ export function SearchFilterButton({
);

return (
<Tooltip content={content} preferredPosition="above" hoverDelay={400}>
<Tooltip
content={content}
preferredPosition="above"
hoverDelay={400}
zIndexOverride={disclosureZIndexOverride}
>
{activator}
</Tooltip>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,19 @@ export enum SortButtonDirection {
export interface SortButtonProps {
choices: SortButtonChoice[];
selected: ChoiceListProps['selected'];
onChange: (selected: string[]) => void;
disabled?: boolean;
disclosureZIndexOverride?: number;
onChange: (selected: string[]) => void;
onChangeKey?: (key: string) => void;
onChangeDirection?: (direction: string) => void;
}

export function SortButton({
choices,
selected,
onChange,
disabled,
disclosureZIndexOverride,
onChange,
onChangeKey,
onChangeDirection,
}: SortButtonProps) {
Expand Down Expand Up @@ -99,6 +101,7 @@ export function SortButton({
content={i18n.translate('Polaris.IndexFilters.SortButton.tooltip')}
preferredPosition="above"
hoverDelay={400}
zIndexOverride={disclosureZIndexOverride}
>
<Button
size="slim"
Expand All @@ -114,12 +117,13 @@ export function SortButton({

return (
<Popover
fluidContent
active={active && !disabled}
activator={sortButton}
autofocusTarget="first-node"
onClose={handleClose}
preferredAlignment="right"
fluidContent
zIndexOverride={disclosureZIndexOverride}
>
<Box
minWidth="148px"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {mountWithApp} from 'tests/utilities';

import {ChoiceList} from '../../../../ChoiceList';
import {Popover} from '../../../../Popover';
import {Tooltip} from '../../../../Tooltip';
import {UnstyledButton} from '../../../../UnstyledButton';
import type {SortButtonChoice} from '../../../types';
import {SortButton} from '..';
Expand Down Expand Up @@ -79,6 +80,36 @@ describe('SortButton', () => {
});
});

it('passes the disclosureZIndexOverride to the popover when provided', () => {
const disclosureZIndexOverride = 517;
const props: SortButtonProps = {
onChange: jest.fn(),
choices,
selected: ['order-number asc'],
disclosureZIndexOverride,
};
const wrapper = mountWithApp(<SortButton {...props} />);

expect(wrapper).toContainReactComponent(Popover, {
zIndexOverride: disclosureZIndexOverride,
});
});

it('passes the disclosureZIndexOverride to the tooltip when provided', () => {
const disclosureZIndexOverride = 517;
const props: SortButtonProps = {
onChange: jest.fn(),
choices,
selected: ['order-number asc'],
disclosureZIndexOverride,
};
const wrapper = mountWithApp(<SortButton {...props} />);

expect(wrapper).toContainReactComponent(Tooltip, {
zIndexOverride: disclosureZIndexOverride,
});
});

it('fires the onChange handler when the ChoiceList changes', () => {
const props: SortButtonProps = {
onChange: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,20 @@ describe('IndexFilters', () => {
});
});

it('passes the disclosureZIndexOverride to the Tabs when provided', () => {
const disclosureZIndexOverride = 517;
const wrapper = mountWithApp(
<IndexFilters
{...defaultProps}
disclosureZIndexOverride={disclosureZIndexOverride}
/>,
);

expect(wrapper).toContainReactComponent(Tabs, {
disclosureZIndexOverride,
});
});

it('renders the SearchFilterButton tooltipContent with keyboard shortcut by default', () => {
const wrapper = mountWithApp(<IndexFilters {...defaultProps} />);

Expand All @@ -149,6 +163,20 @@ describe('IndexFilters', () => {
});
});

it('passes the disclosureZIndexOverride to the SearchFilterButton when provided', () => {
const disclosureZIndexOverride = 517;
const wrapper = mountWithApp(
<IndexFilters
{...defaultProps}
disclosureZIndexOverride={disclosureZIndexOverride}
/>,
);

expect(wrapper).toContainReactComponent(SearchFilterButton, {
disclosureZIndexOverride,
});
});

it('onQueryChange gets called correctly', () => {
const wrapper = mountWithApp(
<IndexFilters {...defaultProps} mode={IndexFiltersMode.Filtering} />,
Expand All @@ -161,13 +189,42 @@ describe('IndexFilters', () => {
expect(defaultProps.onQueryChange).toHaveBeenCalledWith('bar');
});

it('onSort gets called correctly', () => {
const wrapper = mountWithApp(<IndexFilters {...defaultProps} />);
wrapper.act(() => {
wrapper.find(SortButton)!.trigger('onChange', ['customer-name asc']);
describe('sortOptions', () => {
it('does not render the SortButton component when sortOptions is not provided', () => {
const wrapper = mountWithApp(
<IndexFilters {...defaultProps} sortOptions={undefined} />,
);

expect(wrapper).not.toContainReactComponent(SortButton);
});

it('renders the SortButton component when sortOptions is provided', () => {
const wrapper = mountWithApp(<IndexFilters {...defaultProps} />);
expect(wrapper).toContainReactComponent(SortButton);
});

it('passes zIndexOverride to the SortButton component when provided', () => {
const disclosureZIndexOverride = 517;
const wrapper = mountWithApp(
<IndexFilters
{...defaultProps}
disclosureZIndexOverride={disclosureZIndexOverride}
/>,
);

expect(wrapper).toContainReactComponent(SortButton, {
disclosureZIndexOverride,
});
});

expect(defaultProps.onSort).toHaveBeenCalledWith(['customer-name asc']);
it('onSort gets called correctly', () => {
const wrapper = mountWithApp(<IndexFilters {...defaultProps} />);
wrapper.act(() => {
wrapper.find(SortButton)!.trigger('onChange', ['customer-name asc']);
});

expect(defaultProps.onSort).toHaveBeenCalledWith(['customer-name asc']);
});
});

describe('filters', () => {
Expand Down
17 changes: 12 additions & 5 deletions polaris-react/src/components/Tabs/Tabs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,18 +42,20 @@ export interface TabsProps {
selected: number;
/** Whether the Tabs are disabled or not. */
disabled?: boolean;
/** Optional callback invoked when a Tab becomes selected. */
onSelect?: (selectedTabIndex: number) => void;
/** Whether to show the add new view Tab. */
canCreateNewView?: boolean;
/** Label for the new view Tab. Will override the default of "Create new view" */
newViewAccessibilityLabel?: string;
/** Optional callback invoked when a merchant saves a new view from the Modal */
onCreateNewView?: (value: string) => Promise<boolean>;
/** Fit tabs to container */
fitted?: boolean;
/** Text to replace disclosures horizontal dots */
disclosureText?: string;
/** Override z-index of popovers and tooltips */
disclosureZIndexOverride?: number;
/** Optional callback invoked when a Tab becomes selected. */
onSelect?: (selectedTabIndex: number) => void;
/** Optional callback invoked when a merchant saves a new view from the Modal */
onCreateNewView?: (value: string) => Promise<boolean>;
}
const CREATE_NEW_VIEW_ID = 'create-new-view';

Expand All @@ -68,6 +70,7 @@ export const Tabs = ({
onSelect,
fitted,
disclosureText,
disclosureZIndexOverride,
}: TabsProps) => {
const i18n = useI18n();
const {mdDown} = useBreakpoints();
Expand Down Expand Up @@ -205,17 +208,19 @@ export const Tabs = ({
onToggleModal={handleToggleModal}
onTogglePopover={handleTogglePopover}
viewNames={viewNames}
disclosureZIndexOverride={disclosureZIndexOverride}
ref={index === selected ? selectedTabRef : null}
/>
);
},
[
disabled,
handleTabClick,
tabs,
children,
selected,
tabToFocus,
disclosureZIndexOverride,
handleTabClick,
handleToggleModal,
handleTogglePopover,
],
Expand Down Expand Up @@ -578,6 +583,7 @@ export const Tabs = ({
active={disclosureActivatorVisible && showDisclosure}
onClose={handleClose}
autofocusTarget="first-node"
zIndexOverride={disclosureZIndexOverride}
>
<List
focusIndex={hiddenTabs.indexOf(tabToFocus)}
Expand Down Expand Up @@ -608,6 +614,7 @@ export const Tabs = ({
)}
preferredPosition="above"
hoverDelay={400}
zIndexOverride={disclosureZIndexOverride}
>
{newTab}
</Tooltip>
Expand Down
Loading

0 comments on commit a60d8aa

Please sign in to comment.