Skip to content

[TopBar] Add showFocusBorder prop option to TopBar.SearchField #2886

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 20 commits into from
Apr 3, 2020
Merged

[TopBar] Add showFocusBorder prop option to TopBar.SearchField #2886

merged 20 commits into from
Apr 3, 2020

Conversation

alex-page
Copy link
Member

@alex-page alex-page commented Apr 1, 2020 •
edited
Loading

WHY are these changes introduced?

Added showFocusBorder prop to the TopBar.SearchField to allow users to add show a border on focus (https://github.com/Shopify/web/pull/25334).

WHAT is this pull request doing?

A border is added to the backdrop when the search input receives focus. The color of the border is determined by if the user has provided the focusBorder theme value.

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx: 
import React, {useState, useCallback} from 'react';
import {ArrowLeftMinor} from '@shopify/polaris-icons';

import {Page, Frame, TopBar, Card, ActionList, AppProvider} from '../src';

function TopBarExample() {
  const [isUserMenuOpen, setIsUserMenuOpen] = useState(false);
  const [isSearchActive, setIsSearchActive] = useState(false);
  const [searchValue, setSearchValue] = useState('');

  const toggleIsUserMenuOpen = useCallback(
    () => setIsUserMenuOpen((isUserMenuOpen) => !isUserMenuOpen),
    [],
  );

  const handleSearchResultsDismiss = useCallback(() => {
    setIsSearchActive(false);
    setSearchValue('');
  }, []);

  const handleSearchChange = useCallback((value) => {
    setSearchValue(value);
    setIsSearchActive(value.length > 0);
  }, []);

  const handleNavigationToggle = useCallback(() => {
    console.log('toggle navigation visibility');
  }, []);

  const theme = {
    colors: {
      topBar: {
        color: '#212B36',
        background: '#ffffff',
        backgroundLighter: '#DFE3E8',
      },
    },
    logo: {
      width: 124,
      topBarSource:
        'https://cdn.shopify.com/s/files/1/0446/6937/files/jaded-pixel-logo-color.svg?6215648040070010999',
      url: 'http://jadedpixel.com',
      accessibilityLabel: 'Jaded Pixel',
    },
  };

  const userMenuMarkup = (
    <TopBar.UserMenu
      actions={[
        {
          items: [{content: 'Back to Shopify', icon: ArrowLeftMinor}],
        },
        {
          items: [{content: 'Community forums'}],
        },
      ]}
      name="Dharma"
      detail="Jaded Pixel"
      initials="D"
      open={isUserMenuOpen}
      onToggle={toggleIsUserMenuOpen}
    />
  );

  const searchResultsMarkup = (
    <Card>
      <ActionList
        items={[
          {content: 'Shopify help center'},
          {content: 'Community forums'},
        ]}
      />
    </Card>
  );

  const searchFieldMarkup = (
    <TopBar.SearchField
      onChange={handleSearchChange}
      value={searchValue}
      placeholder="Search"
      showFocusBorder
    />
  );

  const topBarMarkup = (
    <TopBar
      showNavigationToggle
      userMenu={userMenuMarkup}
      searchResultsVisible={isSearchActive}
      searchField={searchFieldMarkup}
      searchResults={searchResultsMarkup}
      onSearchResultsDismiss={handleSearchResultsDismiss}
      onNavigationToggle={handleNavigationToggle}
    />
  );

  return (
    <div style={{height: '250px'}}>
      <AppProvider
        theme={theme}
        i18n={{
          Polaris: {
            Avatar: {
              label: 'Avatar',
              labelWithInitials: 'Avatar with initials {initials}',
            },
            Frame: {skipToContent: 'Skip to content'},
            TopBar: {
              toggleMenuLabel: 'Toggle menu',
              SearchField: {
                clearButtonLabel: 'Clear',
                search: 'Search',
              },
            },
          },
        }}
      >
        <Frame topBar={topBarMarkup} />
      </AppProvider>
    </div>
  );
}

export function Playground() {
  return <TopBarExample />;
}

🎩 checklist

@alex-page alex-page requested review from dfmcphee and kyledurand April 1, 2020 22:01
@alex-page alex-page self-assigned this Apr 1, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Apr 1, 2020 •
edited
Loading

🟡 This pull request modifies 9 files and might impact 66 other files. This is an average splash zone for a change, remember to tophat areas that could be affected.

Details:
All files potentially affected (total: 66)
📄 UNRELEASED.md (total: 0)

Files potentially affected (total: 0)

🧩 src/components/ThemeProvider/tests/ThemeProvider.test.tsx (total: 0)

Files potentially affected (total: 0)

📄 src/components/TopBar/README.md (total: 0)

Files potentially affected (total: 0)

🎨 src/components/TopBar/components/SearchField/SearchField.scss (total: 2)

Files potentially affected (total: 2)

🧩 src/components/TopBar/components/SearchField/SearchField.tsx (total: 1)

Files potentially affected (total: 1)

🧩 src/components/TopBar/components/SearchField/tests/SearchField.test.tsx (total: 0)

Files potentially affected (total: 0)

🧩 src/utilities/custom-properties/custom-properties.ts (total: 0)

Files potentially affected (total: 0)

🧩 src/utilities/theme/tests/utils.test.ts (total: 0)

Files potentially affected (total: 0)

🧩 src/utilities/theme/utils.ts (total: 65)

Files potentially affected (total: 65)

@alex-page alex-page changed the title Add focusBorder theme option to TopBar Polaris: Add focusBorder theme option to TopBar Apr 2, 2020
kyledurand
kyledurand approved these changes Apr 2, 2020
View reviewed changes
Copy link
Member

@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.

Code and 🎩 look good 👍

@alex-page
Copy link
Member Author

alex-page commented Apr 2, 2020 •
edited
Loading

Going to rework this a little bit with feedback from @danrosenthal

  • Add a prop to toggle if the border shows on focus
  • In theme/utils.ts add a setBorderColor function to return the correct border color for the TopBar

@kyledurand
Copy link
Member

cc @Shopify/global-nav-web for tracking

@alex-page alex-page requested a review from kyledurand April 2, 2020 21:11
@alex-page alex-page requested a review from danrosenthal April 2, 2020 21:16
kyledurand
kyledurand reviewed Apr 2, 2020
View reviewed changes
Copy link
Member

@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.

I think it would be good to add some tests around setBorderColor

@alex-page alex-page changed the title Polaris: Add focusBorder theme option to TopBar Polaris: Add showFocusBorder prop option to TopBar.SearchField Apr 2, 2020
@alex-page
Copy link
Member Author

I think it would be good to add some tests around setBorderColor

💯

@alex-page alex-page changed the title Polaris: Add showFocusBorder prop option to TopBar.SearchField [TopBar] Add showFocusBorder prop option to TopBar.SearchField Apr 2, 2020
@alex-page alex-page requested a review from kyledurand April 3, 2020 00:01
kyledurand
kyledurand approved these changes Apr 3, 2020
View reviewed changes
Copy link
Member

@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.

Just one super nit pick but I'm not bothered if this gets merged without the change 👍

@kyledurand
Update src/components/TopBar/components/SearchField/tests/SearchField…
ef4be43 
….test.tsx

Co-Authored-By: Kyle Durand <kyledurand@users.noreply.github.com>
@alex-page alex-page merged commit 8832a1a into master Apr 3, 2020
@alex-page alex-page deleted the topbar-focus-border branch April 3, 2020 13:30
athornburg pushed a commit to athornburg/polaris-react that referenced this pull request Apr 15, 2020
@kyledurand @athornburg
[TopBar] Add showFocusBorder prop option to TopBar.SearchField (Shopi…
66b4f78 
…fy#2886)

* Add showFocusBorder prop option to TopBar.SearchField

Co-authored-by: Kyle Durand <kyledurand@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kyledurand kyledurand kyledurand approved these changes

@dfmcphee dfmcphee Awaiting requested review from dfmcphee

@danrosenthal danrosenthal Awaiting requested review from danrosenthal

Assignees

@alex-page alex-page

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@alex-page @kyledurand