Skip to content

Conversation

@sarahill
Copy link
Contributor

@sarahill sarahill commented Oct 17, 2019

WHY are these changes introduced?

Inconsistent alignment of controls between OptionList and ChoiceList items. In OptionList items are center-aligned which causes things to look off when the items wrap.

Fixes #1310

WHAT is this pull request doing?

Adds vertical adjustment to OptionList control items

Before After
Screen Shot 2019-10-17 at 11 12 18 AM Screen Shot 2019-10-17 at 11 11 49 AM

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:
import React, {useCallback, useState} from 'react';
import {Page, Card, OptionList, ChoiceList} from '../src';

export function Playground() {
  const [selected, setSelected] = useState(['hidden']);

  const handleChange = useCallback((value) => setSelected(value), []);

  return (
    <Page title="Playground">
      <Card>
        <OptionList
          title="Inventory Location"
          onChange={setSelected}
          options={[
            {
              value: 'byward_market',
              label:
                'Currently, items that are two lines have the checkboxes vertically aligned to the middle of the text. They should be aligned to the top to be consistent with ChoiceList.',
            },
            {
              value: 'centretown',
              label:
                'Our color usage guidelines indicate that the color Indigo should be used to signify active/highlighted states. Indigo is active and should be shown to indicate active states.',
            },
            {value: 'hintonburg', label: 'Hintonburg'},
            {value: 'westboro', label: 'Westboro'},
            {value: 'downtown', label: 'Downtown'},
          ]}
          selected={selected}
          allowMultiple
        />
      </Card>

      <Card sectioned>
        <ChoiceList
          allowMultiple
          title="While the customer is checking out"
          choices={[
            {
              label:
                'Use the shipping address as the billing address by default',
              value: 'shipping',
              helpText:
                'Reduces the number of fields required to check out. The billing address can still be edited.',
            },
            {
              label:
                'Currently, items that are two lines have the checkboxes vertically aligned to the middle of the text. They should be aligned to the top to be consistent with ChoiceList.',
              value: 'confirmation',
              helpText:
                'Customers must review their order details before purchasing.',
            },
          ]}
          selected={selected}
          onChange={handleChange}
        />
      </Card>
    </Page>
  );
}

🎩 checklist

@github-actions
Copy link
Contributor

github-actions bot commented Oct 17, 2019

💦 Potential splash zone of changes introduced to src/**/*.tsx in this pull request:

Files modified3
Files potentially affected12

Details

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

Files potentially affected (total: 0)

🎨 src/components/Choice/Choice.scss (total: 8)

Files potentially affected (total: 8)

🎨 src/components/OptionList/components/Option/Option.scss (total: 4)

Files potentially affected (total: 4)


This comment automatically updates as changes are made to this pull request.
Feedback, troubleshooting: open an issue or reach out on Slack in #polaris-tooling.

@sarahill sarahill added the Bug Something is broken and not working as intended in the system. label Oct 17, 2019
@sarahill sarahill self-assigned this Oct 17, 2019
@sarahill sarahill requested a review from kaelig October 18, 2019 20:06
Copy link
Contributor

@kaelig kaelig left a comment

Choose a reason for hiding this comment

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

A couple of comments — I haven't tophatted yet.

@sarahill sarahill requested a review from kaelig October 21, 2019 14:27
Copy link
Contributor

@dleroux dleroux left a comment

Choose a reason for hiding this comment

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

🎩 looks good.

@dleroux dleroux force-pushed the checkbox-alignment-choicelist branch from 209b58d to 8225c24 Compare October 22, 2019 18:27
@sarahill sarahill merged commit 854d45d into master Oct 22, 2019
@sarahill sarahill deleted the checkbox-alignment-choicelist branch October 22, 2019 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something is broken and not working as intended in the system.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[OptionList] inconsistent alignment of check boxes with ChoiceList

3 participants