Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Filters] Remove extra margin when rightAction is null #4134

Merged
merged 1 commit into from
Apr 21, 2021

Conversation

alololox
Copy link
Contributor

@alololox alololox commented Apr 20, 2021

WHY are these changes introduced?

When the ResourceList & Filter components are used in either an empty state configuration, or one where no filters are passed in (i.e. just search via TextField), a class with extra right margin is added causing the input to appear misaligned. This can be seen on the "empty state resource list" story/example linked in the docs: https://codesandbox.io/s/uyg6i?module=App.js

Currently, with margin: Empty state: Screen Shot 2021-04-20 at 10 46 23 AM Existing web example:
This PR proposes: Empty state: Screen Shot 2021-04-20 at 10 46 50 AM

WHAT is this pull request doing?

Sets rightActionMarkup in Filters.tsx to null when no filters are passed in, which prevents appending an empty div with the offending classes in ConnectedFilterControl here.

How to 馃帺

Either copy paste the following into the Playground example or run through all existing ResourceList and Filter examples.

Copy-paste this code in playground/Playground.tsx:
import React, {useState} from 'react';

import {
  Page,
  Card,
  ResourceList,
  ResourceItem,
  Filters,
  TextStyle,
} from '../src';

export function Playground() {
  const [queryValue, setQueryValue] = useState(null);
  const handleClearAll = () => {
    setQueryValue(null);
  };

  const resourceName = {
    singular: 'customer',
    plural: 'customers',
  };

  const items = [
    {
      id: 108,
      url: 'customers/341',
      name: 'Mae Jemison',
      location: 'Decatur, USA',
    },
    {
      id: 208,
      url: 'customers/256',
      name: 'Ellen Ochoa',
      location: 'Los Angeles, USA',
    },
  ];

  const filters = [];

  const appliedFilters = [];

  const filterControl = (
    <Filters
      queryValue={queryValue}
      filters={filters}
      appliedFilters={appliedFilters}
      onQueryChange={setQueryValue}
      onQueryClear={() => setQueryValue(null)}
      onClearAll={handleClearAll}
    />
  );

  return (
    <Page>
      <Card>
        <ResourceList
          resourceName={resourceName}
          items={items}
          renderItem={renderItem}
          filterControl={filterControl}
        />
      </Card>
    </Page>
  );

  function renderItem(item) {
    const {id, url, name, location} = item;
    return (
      <ResourceItem id={id} url={url}>
        <div>
          {name}: {location}
        </div>
      </ResourceItem>
    );
  }
}

馃帺 checklist

  • Tested on mobile
  • Tested on multiple browsers
  • Tested for accessibility
  • Updated the component's README.md with documentation changes
  • Tophatted documentation changes in the style guide
  • For visual design changes, pinged one of @鈥奌YPD, @鈥妋irualves, @鈥妔arahill, or @鈥妑y5n to update the Polaris UI kit

@alololox alololox force-pushed the filters/no-rightaction-no-margin branch from 14a055e to ef3a235 Compare April 20, 2021 19:26
@github-actions
Copy link
Contributor

github-actions bot commented Apr 20, 2021

馃煝 No significant changes to src/**/*.tsx were detected.

@alololox alololox marked this pull request as ready for review April 20, 2021 21:49
@alololox alololox changed the title [Filters] Set actionRight to null when filters are empty [Filters] Remove extra margin when rightAction is null Apr 20, 2021
@alololox alololox force-pushed the filters/no-rightaction-no-margin branch from ef3a235 to aa9090e Compare April 21, 2021 16:40
Copy link
Contributor

@kyledurand kyledurand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the tests 馃檶

Copy link
Member

@pedrodurek pedrodurek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 馃帀

@alololox alololox merged commit 5e44bf0 into main Apr 21, 2021
@alololox alololox deleted the filters/no-rightaction-no-margin branch April 21, 2021 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants