Skip to content

Conversation

@ruairiphackett
Copy link
Contributor

@ruairiphackett ruairiphackett commented Aug 28, 2019

WHY are these changes introduced?

Fixes #1345

WHAT is this pull request doing?

Removes a right value in the ResourceList.Item Actions which was causing it to not be aligned with other right aligned buttons like the Card actions:

Before:

https://screenshot.click/28-08-f7fkp-bn9nb.png

After:

https://screenshot.click/28-06-7lq9b-j7fpt.png

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:
import React from 'react';
import {Page, Card, TextStyle, Avatar, ResourceList, Button} from '../src';

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

interface State {}
interface Props {}

class MyPlayground extends React.Component<Props, State> {
  render() {
    const divStyle = {
      height: '200px',
      backgroundColor: 'red',
    };
    const parentStyle = {
      height: '200px',
      backgroundColor: 'blue',
    };
    return (
      <Page title="Playground">
        <Card title="Active customers" actions={[{content: 'Add customer'}]}>
          <Card.Section>
            <TextStyle variation="subdued">
              Customers that have made a purchase in the last 2 weeks
            </TextStyle>
          </Card.Section>
          <ResourceList
            resourceName={{singular: 'customer', plural: 'customers'}}
            items={[
              {
                id: 341,
                url: 'customers/341',
                name: 'Mae Jemison',
                location: 'Decatur, USA',
                latestOrderUrl: 'orders/1456',
              },
              {
                id: 256,
                url: 'customers/256',
                name: 'Ellen Ochoa',
                location: 'Los Angeles, USA',
                latestOrderUrl: 'orders/1457',
              },
            ]}
            renderItem={(item) => {
              const {id, url, name, location, latestOrderUrl} = item;
              const media = <Avatar customer size="medium" name={name} />;
              const shortcutActions = latestOrderUrl
                ? [{content: 'View latest order', url: latestOrderUrl}]
                : null;

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

🎩 checklist

@ruairiphackett ruairiphackett changed the title [ResourceList] Fix Item right position [ResourceList] Fix Item Action right position Aug 28, 2019
@ruairiphackett ruairiphackett force-pushed the resource-list-item-padding branch from c8eba7b to ccd11d0 Compare August 29, 2019 09:54
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.

@ruairiphackett thanks for doing this. However the Actions targetted also affect the actions when they are not persistent with persistActions so it ends up looking like this on hover:

Screen Shot 2019-08-30 at 9 17 36 AM

Setting right: auto on persistent actions only might do the trick.

With your change, it still doesn't line up properly. I inspected it and it looks like the card is using just a plain button and the persistent Actions are using plain and slim. Removing slim on persistent actions only should make this line up.

There's also a UNRELEASED conflict.

@ruairiphackett ruairiphackett force-pushed the resource-list-item-padding branch from ccd11d0 to 5e806bc Compare September 2, 2019 10:24
@ruairiphackett
Copy link
Contributor Author

ruairiphackett commented Sep 2, 2019

Thanks @dleroux I should have noticed that! 🤦‍♂

I removed slim on persistent actions & that did the trick 👍

I tried with right: auto; on persistent actions, but was seeing the same issue as you saw here.

I noticed there was also an issue with non-persistent actions not lining up:

https://screenshot.click/02-06-26tfo-28nfz.png

With this in mind I went with removing the right value from Actions and adding the new right: spacing(loose); value to non-persistent actions when the resource item is hovered (to match the padding for the item in the card header).

I noticed that the non-persistent action should probably line up with the header alternate tool button or sorting instead of the actions in the header, so I added a further change to match this:

https://screenshot.click/02-39-f2jfu-e3k1m.png

Before this PR:

https://screenshot.click/02-43-s1sh3-vftem.png

If you think I should do this change in another PR to keep the change separate or if this is unnecessary please let me know.

@dleroux
Copy link
Contributor

dleroux commented Sep 5, 2019

The joys of Resource List :). I just noticed that when the actions get hidden on a smaller screen, the Disclosure button (3 little dots on the popover) need to be updated as well. I think all you'll need is remove this:

@include breakpoint-after(resource-list-item(breakpoint-small)) {

Screen Shot 2019-09-05 at 4 50 35 PM

Once that is done, squashed and conflicts resolved, I think we're good.

@dleroux dleroux self-requested a review September 5, 2019 20:51
@ruairiphackett ruairiphackett force-pushed the resource-list-item-padding branch from 66a3555 to 3733cf9 Compare September 6, 2019 13:46
@ruairiphackett
Copy link
Contributor Author

@dleroux Made that change and resolved the UNRELEASED.md conflict 👍

@ruairiphackett ruairiphackett force-pushed the resource-list-item-padding branch from 3733cf9 to a981236 Compare September 9, 2019 09:49
@ruairiphackett ruairiphackett force-pushed the resource-list-item-padding branch from a981236 to 0307dd1 Compare September 9, 2019 09:57
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.

Thank you for doing this! 🚢

@ruairiphackett ruairiphackett merged commit 7729fb4 into master Sep 9, 2019
@ruairiphackett ruairiphackett deleted the resource-list-item-padding branch September 9, 2019 15:08
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.

Spacing on ResourceList.Item actions isn't the same as Card actions

2 participants