[Filters] Fix FilterTrigger class to clear margins

[shermanhui]

WHY are these changes introduced?

Fixes #2286

On Safari, there is 2px of margin-top and margin-bottom being applied to the FilterTrigger button in the Filter component.

WHAT is this pull request doing?

• Added a rule to explicitly set margin to 0 in the FilterTrigger class
  since there is already no margin on Chrome/FireFox, it seemed like a
  sensible solution to explicitly ensure there is no margin applied regardless
  of browser.
• It seemed like overkill to use the unstyled-button
  mixin so I opted for the single margin rule,
  but it could make sense to use the mixin since we want to reset this
  button element to an 'unstyled' state.

Question for Code Owners:

What would be the best practice for this Design System in this case? To use the unstyled-mixin and removing the duplicated rules such as background and border or just adding the margin rule like I've done in this PR?

In the case of using the unstyled-mixin the browser will warn that there are duplicate properties and I'm not sure if that's following best practices for this project. i.e in the screenshot below padding is duplicated as a result of using unstyled-mixin after removing background and border.

Before/After Images

Before:

After:

Copy-paste this code in playground/Playground.tsx:

import React, {useCallback, useState} from 'react';
import {
  ChoiceList,
  Page,
  Filters,
  TextField,
  TextStyle,
  Card,
  ResourceList,
  Button,
  Avatar,
} from '../src';

export function Playground() {
const [availability, setAvailability] = useState(null);
  const [productType, setProductType] = useState(null);
  const [taggedWith, setTaggedWith] = useState(null);
  const [queryValue, setQueryValue] = useState(null);

  const handleAvailabilityChange = useCallback(
    (value) => setAvailability(value),
    [],
  );
  const handleProductTypeChange = useCallback(
    (value) => setProductType(value),
    [],
  );
  const handleTaggedWithChange = useCallback(
    (value) => setTaggedWith(value),
    [],
  );
  const handleQueryValueChange = useCallback(
    (value) => setQueryValue(value),
    [],
  );
  const handleAvailabilityRemove = useCallback(() => setAvailability(null), []);
  const handleProductTypeRemove = useCallback(() => setProductType(null), []);
  const handleTaggedWithRemove = useCallback(() => setTaggedWith(null), []);
  const handleQueryValueRemove = useCallback(() => setQueryValue(null), []);

  const handleClearAll = useCallback(() => {
    handleAvailabilityRemove();
    handleProductTypeRemove();
    handleTaggedWithRemove();
    handleQueryValueRemove();
  }, [
    handleAvailabilityRemove,
    handleProductTypeRemove,
    handleQueryValueRemove,
    handleTaggedWithRemove,
  ]);

  const filters = [
    {
      key: 'taggedWith',
      label: 'Tagged with',
      filter: (
        <TextField
          label="Tagged with"
          value={taggedWith}
          onChange={handleTaggedWithChange}
          labelHidden
        />
      ),
      shortcut: true,
    },
    {
      key: 'availability',
      label: 'Availability',
      filter: (
        <ChoiceList
          title="Availability"
          titleHidden
          choices={[
            {label: 'Online Store', value: 'Online Store'},
            {label: 'Point of Sale', value: 'Point of Sale'},
            {label: 'Buy Button', value: 'Buy Button'},
          ]}
          selected={availability || []}
          onChange={handleAvailabilityChange}
          allowMultiple
        />
      ),
      shortcut: true,
    },
    {
      key: 'productType',
      label: 'Product type',
      filter: (
        <ChoiceList
          title="Product type"
          titleHidden
          choices={[
            {label: 'T-Shirt', value: 'T-Shirt'},
            {label: 'Accessory', value: 'Accessory'},
            {label: 'Gift card', value: 'Gift card'},
          ]}
          selected={productType || []}
          onChange={handleProductTypeChange}
          allowMultiple
        />
      ),
    },
  ];

  const appliedFilters = !isEmpty(taggedWith)
    ? [
        {
          key: 'taggedWith',
          label: disambiguateLabel('taggedWith', taggedWith),
          onRemove: handleTaggedWithRemove,
        },
      ]
    : [];

  function disambiguateLabel(key, value) {
    switch (key) {
      case 'taggedWith':
        return `Tagged with ${value}`;
      default:
        return value;
    }
  }

  function isEmpty(value) {
    if (Array.isArray(value)) {
      return value.length === 0;
    } else {
      return value === '' || value == null;
    }
  }

  return (
    <Page title="Playground">
      <div style={{height: '568px'}}>
        <Card>
          <ResourceList
            resourceName={{singular: 'customer', plural: 'customers'}}
            filterControl={
              <Filters
                queryValue={queryValue}
                filters={filters}
                appliedFilters={appliedFilters}
                onQueryChange={handleQueryValueChange}
                onQueryClear={handleQueryValueRemove}
                onClearAll={handleClearAll}
              >
                <div style={{paddingLeft: '8px'}}>
                  <Button onClick={() => console.log('New filter saved')}>
                    Save
                  </Button>
                </div>
              </Filters>
            }
            items={[
              {
                id: 341,
                url: 'customers/341',
                name: 'Mae Jemison',
                location: 'Decatur, USA',
              },
              {
                id: 256,
                url: 'customers/256',
                name: 'Ellen Ochoa',
                location: 'Los Angeles, USA',
              },
            ]}
            renderItem={(item) => {
              const {id, url, name, location} = item;
              const media = <Avatar customer size="medium" name={name} />;

              return (
                <ResourceList.Item
                  id={id}
                  url={url}
                  media={media}
                  accessibilityLabel={`View details for ${name}`}
                >
                  <h3>
                    <TextStyle variation="strong">{name}</TextStyle>
                  </h3>
                  <div>{location}</div>
                </ResourceList.Item>
              );
            }}
          />
        </Card>
      </div>
    </Page>
  );
}

🎩 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 @ HYPD, @ mirualves, @ sarahill, or @ ry5n to update the Polaris UI kit

[shermanhui]

While creating a fix for this issue, I discovered that the :focus state was not being applied correctly in Safari and FireFox due to the way the browsers handle clicks. I need some advice as to how the team would like this issue to be resolved as it could affect other :focus related functionalities across the component library! I've created an issue #2293.

[dleroux]

Suggested change

	on the button element on Safari ([2292](https://github.com/Shopify/polaris-react/pull/2292))
	on the button element on Safari ([#2292](https://github.com/Shopify/polaris-react/pull/2292))

[dleroux]

Thanks for doing this @shui91. To answer your question, I think what you did here is the best approach. No need to use unstyled-mixin since all that is needed is 1 line of CSS. Thanks for asking. I'll take a look at your other issue. You'll need to update the conflict in UNRELEASED.md, once you do that ping me and I will approve it.

[shermanhui]

@dleroux thanks for following up on this and the comment on the other issue! I'll make another PR for that one soon :) Appreciate the help!

[dleroux]

@shui91 sorry about that, there was a release last week, therefore, more conflicts on the unreleased.md.

[shermanhui]

@dleroux resolved

[dleroux]

@shui91 looks like there are still some entries that we're in the previous release.

[shermanhui]

@dleroux Happy New Year! I've fixed the merge conflicts by rebasing master onto my branch. Hope this will do it now :)

[dleroux]

@shui91 Happy new year to you!

Unfortunately, old entries are still in unreleased.md https://github.com/Shopify/polaris-react/pull/2292/files#diff-1f9a24a2e063326eff13af43500f8b74.

The only change in there should be yours. Sorry to do this to you, we really do appreciate the contribution! Thanks again.

[shermanhui]

https://github.com/Shopify/polaris-react/pull/2292/files#diff-1f9a24a2e063326eff13af43500f8b74

Ohhh gotcha! I didn't catch that the other fixes had already been released. 😅 UNRELEASED.md should now only have this fix now :) Thanks for the clarification @dleroux !

[dleroux]

🎉 🙇 @shui91 Thank You!