Skip to content
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.

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

Add Tiles component #1787

Closed

Conversation

tanhengyeow
Copy link
Contributor

@tanhengyeow tanhengyeow commented Mar 4, 2019

Fixes #1363

Hi @interactivellama ! Feedback for the initial draft would be appreciated :)

Tiles:

const propTypes = {
	/**
	 * HTML title attribute. _Tested with snapshot testing._
	 */
	title: PropTypes.string,
	/**
	 * Action overflow menu dropdown for the tile.
	 */
	dropdownButton: PropTypes.node,
	/**
	 * Different types of tile formats.
	 */
	variant: PropTypes.oneOf(['media', 'board']),
	/**
	 * The content for the Tile component.
	 */
	children: PropTypes.node.isRequired,
};

TileMediaBody:

const propTypes = {
	/**
	 * HTML title attribute. _Tested with snapshot testing._
	 */
	title: PropTypes.string.isRequired,
	/**
	 * The children for the TileMediaBody component. _Tested with Mocha framework and snapshot testing._
	 */
	children: PropTypes.node.isRequired,
};

TileMediaFigure:

const propTypes = {
	/**
	 * The media figure for this tile. It can be an icon, avatar or a task.
	 */
	mediaFigure: PropTypes.node.isRequired,
};

An example that incorporates all the sub components can be seen here.

Some screenshots for reference below.

Default:
image

Default with actions:
image

Icon:
image

Avatar:
image

Task:
image

Article:
image

Board:
image

Additional description


CONTRIBUTOR checklist (do not remove)

Please complete for every pull request

  • First-time contributors should sign the Contributor License Agreement. It's a fancy way of saying that you are giving away your contribution to this project. If you haven't before, wait a few minutes and a bot will comment on this pull request with instructions.
  • npm run lint:fix has been run and linting passes.
  • Mocha, Jest (Storyshots), and components/component-docs.json CI tests pass (npm test).
  • Tests have been added for new props to prevent regressions in the future. See readme.
  • Review the appropriate Storybook stories. Open http://localhost:9001/.
  • The Accessibility panel of each Storybook story has 0 violations (aXe). Open http://localhost:9001/.
  • Review tests are passing in the browser. Open http://localhost:8001/.
  • Review markup conforms to SLDS by looking at DOM snapshot strings.

REVIEWER checklist (do not remove)

  • TravisCI tests pass. This includes linting, Mocha, Jest, Storyshots, and components/component-docs.json tests.
  • Tests have been added for new props to prevent regressions in the future. See readme.
  • Review the appropriate Storybook stories. Open http://localhost:9001/.
  • The Accessibility panel of each Storybook story has 0 violations (aXe). Open http://localhost:9001/.
  • Review tests are passing in the browser. Open http://localhost:8001/.
  • Review markup conforms to SLDS by looking at DOM snapshot strings.
Required only if there are markup / UX changes
  • Add year-first date and commit SHA to last-slds-markup-review in package.json and push.
  • Request a review of the deployed Heroku app by the Salesforce UX Accessibility Team.
  • Add year-first review date, and commit SHA, last-accessibility-review, to package.json and push.
  • While the contributor's branch is checked out, run npm run local-update within locally cloned site repo to confirm the site will function correctly at the next release.

@interactivellama
Copy link
Contributor

Initial thoughts:

dropdownButton should take a full dropdown component.

There is an existing record detail label/content prop in PageHeader.

https://react.lightningdesignsystem.com/components/page-headers/

const details = [
			{
				label: 'Field 1',
				content:
					'Description that demonstrates truncation with content. Description that demonstrates truncation with content.',
			},
			{ label: 'Field 2', content: 'Multiple Values' },
			{
				label: 'Field 3',
				content: <a href="javascript:void(0);">Hyperlink</a>,
			},
			{
				label: 'Field 4',
				content: 'Description (2-line truncation)',
			},
		];

It's more of a collection/options array.

I will come back to this and look at it more later this week.

Copy link
Contributor

@interactivellama interactivellama left a comment

Choose a reason for hiding this comment

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

Re-use of Icon component in media figure is good. I'm open to use of children instead of a data collection (array of objects). We need to allow the consumer to write less classnames, since that's one of the goals of this project.

@interactivellama
Copy link
Contributor

@tanhengyeow Based on our LinkedIn messaging, I'm going to close this pull request. Feel free to re-open at any time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Tile component
2 participants