Add ActivityCard component #86

ryelle · 2018-05-24T16:16:06Z

Fixes #48 – this PR adds a new component ActivityCard. You can check out the README for more details. I've also temporarily added an ActivityList component into the sidebar, this might stay or not depending on how we fetch the activities, I think. So you can see 3 demo activities in the sidebar:

To test

Check out the sidebar on any page
Should load the card as described in the issue/lofi designs

justinshreve

A few minor comments/questions. This is looking really great though (both code + and the demo).

justinshreve · 2018-05-24T18:07:56Z

client/components/activity-card/index.js

+class ActivityCard extends Component {
+	render() {
+		const { actions, date, icon, image, label, menu, children } = this.props;
+		const className = classnames( 'woo-dash__activity-card' );


Do we want to accept an optional className prop? Otherwise you could just pass in the name directly below and not need to import classnames.

I've added a className prop but we still might not need classnames… I think i'm going to leave it as-is in case we need it for the hifi design (if we need to add classes depending on what props are used, for example).

justinshreve · 2018-05-24T18:12:04Z

client/stylesheets/_mixins.scss

+	}
+}
+
+@mixin hover-state {


justinshreve · 2018-05-24T18:15:37Z

client/components/activity-card/README.md

+* `label`: A title for this card (required).
+* `children`: Content used in the body of the action card (required).
+* `actions`: A list of links or buttons shown in the footer of the card.
+* `date`: The timestamp associated with this activity.


What format do you think we'll expect for date here? I see the @TODO Use @wordpress/date to format the date comment below, so will this be the actual date from the API and the component will return the relative time?

It can depend on what the API returns, I guess -- I was thinking it would be just a timestamp, and we would convert to a relative time in the component. moment can accept a lot of different date formats, but most of the other dates in API responses are like 2018-04-24T14:44:15.

justinshreve · 2018-05-24T18:25:43Z

client/layout/sidebar/activity.js

+					menu={ exampleEllipsisMenu }
+					actions={ [ <a href="/">View referral analytics</a> ] }
+					image={
+						<img src="https://vagrant.local/content/uploads/2017/05/beanie-150x150.jpg" alt="" />


Minor since this is just an example, but can we link to a .com image or something? Otherwise the console will throw a 404 (and no demo image).

Yep, I meant to swap that out before making the PR.... then forgot. Thanks for catching.

justinshreve · 2018-05-24T18:31:24Z

client/components/activity-card/index.js

+
+ActivityCard.propTypes = {
+	actions: PropTypes.oneOfType( [ PropTypes.array, PropTypes.element ] ),
+	children: PropTypes.node,


This should have an .isRequired since I don't think we'll want empty cards (and it's documented as required in the README).

timmyc

This is looking good and testing out nicely for me - nice work!

timmyc · 2018-05-24T21:52:35Z

client/components/activity-card/README.md

+  return (
+    <ActivityCard
+      label="Insight"
+      icon={ <Dashicon icon="search" /> }


I like the flexibility of allowing an img or dashicon component here and in the image below - nice!

timmyc · 2018-05-24T21:55:36Z

client/layout/sidebar/activity.js

+				<ActivityCard
+					label="Insight"
+					icon={ <Dashicon icon="search" /> }
+					date="30 minutes ago"


Building off @justinshreve's question above about date format - i could see the date coming in as a timestamp and using moment.js to do "time ago in words" here.

Yep, that's the idea 🙂 using moment from @wordpress/date so that we can get the correct timezone settings.

timmyc · 2018-05-24T22:15:52Z

/cc @jameskoster @josemarques for some design 👀

jameskoster · 2018-05-25T09:48:42Z

Looks good to me.

We're using Gridicons in our mocks but a different set is being used here. I assume this set is already available so it makes sense to use it, I'm just concerned we'll run in to an issue where a corresponding icon isn't available. Is it a big pain to load Gridicons?

ryelle · 2018-05-25T14:22:22Z

We're using Gridicons in our mocks but a different set is being used here

I was wondering if that was going to come up soon :) I was planning on asking about it next meeting. We're using dashicons because that's what core is using, and so we can use their components (like <Dashicon /> and <IconButton />) without jumping through hoops to use our own icons.

Here's a dashicons resource page, though AFAIK it's not totally up to date with everything added for Gutenberg. Checking out the list of SVGs is a better reference.

- Add className prop - children propType is required - Link to a real image

ryelle · 2018-05-25T14:54:17Z

@timmyc @justinshreve I've made updates based on your feedback - thanks for the review 🙂

timmyc

Changes look good to me 👍

ryelle added 4 commits May 24, 2018 11:53

Add an ActivityCard component

98cff12

Add font-size mixin, hover-state mixin, and a fake base padding

7eac9c2

Style the ActivityCard

f17b570

Style the card image

1c65ae6

ryelle added [Status] Needs Review labels May 24, 2018

ryelle self-assigned this May 24, 2018

ryelle requested review from timmyc, justinshreve and psealock May 24, 2018 16:16

justinshreve reviewed May 24, 2018

View reviewed changes

timmyc reviewed May 24, 2018

View reviewed changes

timmyc added this to Needs Review in Isotope May 24, 2018

ryelle added 2 commits May 25, 2018 10:38

Add README

c98843f

Address PR feedback

fdec1d4

- Add className prop - children propType is required - Link to a real image

ryelle force-pushed the add/component-activity-card branch from a05e2ac to fdec1d4 Compare May 25, 2018 14:54

timmyc approved these changes May 25, 2018

View reviewed changes

timmyc added [Status] Ready to Merge and removed [Status] Needs Review labels May 25, 2018

ryelle merged commit 67c874f into master May 25, 2018

Isotope automation moved this from Needs Review to Done 2018-05-21 to 2018-05-27 May 25, 2018

ryelle deleted the add/component-activity-card branch May 25, 2018 19:34

ryelle mentioned this pull request May 25, 2018

CSS Layout: Consistent padding #64

Closed

timmyc mentioned this pull request May 25, 2018

Activity: Create Order Activity Feed #89

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ActivityCard component #86

Add ActivityCard component #86

ryelle commented May 24, 2018 •

edited

Loading

justinshreve left a comment

justinshreve May 24, 2018

ryelle May 25, 2018

justinshreve May 24, 2018

justinshreve May 24, 2018

ryelle May 25, 2018

justinshreve May 24, 2018

ryelle May 25, 2018

justinshreve May 24, 2018

timmyc left a comment

timmyc May 24, 2018

timmyc May 24, 2018

ryelle May 25, 2018

timmyc commented May 24, 2018

jameskoster commented May 25, 2018

ryelle commented May 25, 2018

ryelle commented May 25, 2018

timmyc left a comment

Add ActivityCard component #86

Add ActivityCard component #86

Conversation

ryelle commented May 24, 2018 • edited Loading

justinshreve left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timmyc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timmyc commented May 24, 2018

jameskoster commented May 25, 2018

ryelle commented May 25, 2018

ryelle commented May 25, 2018

timmyc left a comment

Choose a reason for hiding this comment

ryelle commented May 24, 2018 •

edited

Loading