Add Scoped Notification Component #1785

aswinshenoy · 2019-03-04T10:07:17Z

Fixes #1738
Add Scoped Notification Component

@interactivellama @futuremint, kindly review it, I will make any changes required.

Additional description

The Scoped Navigation component as found in SLDS has been ported to react, with almost all its functionality, and reusing the same classes.

Props Taken

className - CSS classes to be added to tag with .slds-scoped-notification.
iconName - Icon for the Scoped Notification
theme - Either Dark or Light

Screenshots

Light Variant

Dark Variant

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 · 2019-03-10T06:43:29Z

Props approved.

icon is not needed, but you can add an override if you'd like to. As far as I am aware of, the UX pattern is to always have an info icon.
Probably just a typo, but className should be on slds-scoped-notification.

aswinshenoy · 2019-03-18T08:53:33Z

@interactivellama I have made the necessary changes making the Icon prop optional, and setting info icon as default.

…-notification

interactivellama

This good is almost done. There are a few small changes to make and I should be able to merge.

interactivellama · 2019-03-21T05:24:08Z

components/scoped-notification/index.jsx

+					`slds-scoped-notification`,
+					`slds-media`,
+					`slds-media_center`,
+					`slds-scoped-notification_${this.props.theme}`,


CSS class names should not be concatenated. Please list out object keys with boolean values https://github.com/salesforce/design-system-react/blob/master/docs/codebase-overview.md#use-classnames-for-conditional-css-classes

interactivellama · 2019-03-21T05:26:54Z

components/scoped-notification/__examples__/light.jsx

+	render() {
+		return (
+			<div
+				style={{


Please remove extra styling.

I have removed extra styling from both light and dark examples

interactivellama · 2019-03-21T05:28:29Z

components/scoped-notification/index.jsx

+// Implements the [Scoped Notification design pattern](https://lightningdesignsystem.com/components/scoped-notifications/) in React.
+// Based on SLDS v2.4.5
+import IconSettings from '~/components/icon-settings';
+import Icon from '~/components/icon';


The ~ import prefix should only be used in examples and not in components. I'm not sure what is documented anyway though. It's in the examples because a regular expression transform is run on them in the doc website.

I have replaced those with

import IconSettings from '../icon-settings'; import Icon from '../icon';

…-notification

interactivellama · 2019-04-26T07:34:31Z

components/scoped-notification/index.jsx

+	/**
+	 *  Icon for the scoped notification
+	 */
+	icon: PropTypes.string,


Icon would the an <Icon> component. This should probably be iconName and folks should know that it's limited to the utility set of icons at this point. https://www.lightningdesignsystem.com/icons/#utility

Ya, True. I went for a rather generic name, thanks for patching it up. I will take a note of it, and would update for the other prop proposals I have made elsewhere as well.

interactivellama · 2019-04-26T07:35:55Z

This pull request does not have any tests. Since the component is not interactive Jest snapshots would be enough.

…to pr/aswinshenoy/1785 # Conflicts: # utilities/constants.js

interactivellama · 2019-04-26T07:40:27Z

components/scoped-notification/index.jsx

+				role="status"
+			>
+				<div className="slds-media__figure">
+					<IconSettings iconPath="/assets/icons">


This line would limit all uses of the component to use this path. This should be removed. IconSettings should only be used outside a component by the consumer to allow customization.

Oh true. I didn't think about it, thank you for fixing it :)

interactivellama · 2019-04-26T07:45:43Z

components/scoped-notification/index.jsx

+			>
+				<div className="slds-media__figure">
+					<IconSettings iconPath="/assets/icons">
+						<Icon


This icon isn't using slds-icon-text-default classname which would make it darker. It's light here in Storybook, but dark on the SLDS website https://www.lightningdesignsystem.com/components/scoped-notifications/

Sorry again, I missed out that. Thank you for fixing it, and in the case in future, if you are engaged, I would be more than happy to patch these reviews comments :)

update icon to iconName since it is not an Icon component

…ystem-react into pr/aswinshenoy/1785 # Conflicts: # components/scoped-notification/__examples__/dark.jsx # components/scoped-notification/__examples__/light.jsx # components/scoped-notification/index.jsx

…ystem-react into pr/aswinshenoy/1785

aswinshenoy

I was about to send in the same lint patch, but I find it still not passing npm run lint.
It says something wrong with prettier, I tried npm run lint:fix, but the only thing that changed was the same as diff in your patch.
Is that okay?

interactivellama · 2019-04-26T08:45:41Z

Those CI output lines look correct. What issues are you seeing?

aswinshenoy · 2019-04-26T08:51:56Z

Those CI output lines look correct. What issues are you seeing?

It seems I misunderstood this, I thought the Travis was failing whenever these messages came. Sorry about those, hehe.

On a closer look, the exit return code says it all :) I shouldn't have bothered 👍 Stupid me.

Add Scoped Notification Component

1f5eaae

salesforce-cla bot added the cla:signed label Mar 4, 2019

Correct Typo in Comment

a72a80d

Update Prop Icon to Optional, with info as Default

450de84

aswinshenoy added 2 commits March 18, 2019 14:24

Merge remote-tracking branch 'origin/scoped-notification' into scoped…

d884351

…-notification

Fix typo

17e0af9

interactivellama self-requested a review March 21, 2019 05:18

interactivellama requested changes Mar 21, 2019

View reviewed changes

aswinshenoy added 2 commits March 21, 2019 17:33

Update with Review Suggestions

a6f0059

Merge remote-tracking branch 'origin/scoped-notification' into scoped…

e5528d0

…-notification

interactivellama added this to In progress in 2019-03 (March) via automation Mar 28, 2019

interactivellama added this to In progress in Clear out PRs on roadmap via automation Apr 16, 2019

pulkonet mentioned this pull request Apr 21, 2019

Components Checklist: Exhaustive checklist of components to be added to align DSR with SLDS #1933

Closed

interactivellama added this to the July 2019 Release cycle milestone Apr 25, 2019

interactivellama reviewed Apr 26, 2019

View reviewed changes

Merge branch 'master' of github.com:salesforce/design-system-react in…

b891249

…to pr/aswinshenoy/1785 # Conflicts: # utilities/constants.js

interactivellama reviewed Apr 26, 2019

View reviewed changes

interactivellama added 3 commits April 26, 2019 01:48

Scoped Notifications: Move IconSettings, add iconName

86f2abf

update icon to iconName since it is not an Icon component

ScopedNotification: Add snapshots

7098b5c

Merge branch 'scoped-notification' of github.com:aswinshenoy/design-s…

534a17e

…ystem-react into pr/aswinshenoy/1785 # Conflicts: # components/scoped-notification/__examples__/dark.jsx # components/scoped-notification/__examples__/light.jsx # components/scoped-notification/index.jsx

interactivellama added this to In progress in 2019-04 (April) via automation Apr 26, 2019

interactivellama added 4 commits April 26, 2019 02:14

ScopedNotification: Remove IconSettings import

a40de47

ScopedNotification: Update Snapshots

a704abd

Merge branch 'scoped-notification' of github.com:aswinshenoy/design-s…

741b71b

…ystem-react into pr/aswinshenoy/1785

Scoped Notification: lint

c9b3839

aswinshenoy commented Apr 26, 2019

View reviewed changes

Scoped Notification: update snapshot from lint

de76dcb

aswinshenoy mentioned this pull request Apr 26, 2019

Add Progress Bar Component #1773

Merged

18 tasks

interactivellama self-assigned this May 2, 2019

interactivellama previously approved these changes May 2, 2019

View reviewed changes

Scoped Notification: Add to site stories

7272a74

interactivellama dismissed their stale review via 7272a74 May 2, 2019 23:19

interactivellama added the scoped-notifications label May 3, 2019

interactivellama approved these changes May 3, 2019

View reviewed changes

interactivellama merged commit 958ace3 into salesforce:master May 3, 2019

Clear out PRs on roadmap automation moved this from In progress to Done May 3, 2019

2019-04 (April) automation moved this from In progress to Done May 3, 2019

interactivellama added this to Reviewer approved in GSoC2019 Evaluation-1 May 27, 2019

interactivellama moved this from Reviewer approved to Done in GSoC2019 Evaluation-1 May 27, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Scoped Notification Component #1785

Add Scoped Notification Component #1785

aswinshenoy commented Mar 4, 2019 •

edited

interactivellama commented Mar 10, 2019

aswinshenoy commented Mar 18, 2019

interactivellama left a comment

interactivellama Mar 21, 2019

interactivellama Mar 21, 2019

aswinshenoy Mar 21, 2019

interactivellama Mar 21, 2019

aswinshenoy Mar 21, 2019

interactivellama Apr 26, 2019

aswinshenoy Apr 26, 2019

interactivellama commented Apr 26, 2019

interactivellama Apr 26, 2019

aswinshenoy Apr 26, 2019

interactivellama Apr 26, 2019

aswinshenoy Apr 26, 2019

aswinshenoy left a comment •

edited

interactivellama commented Apr 26, 2019

aswinshenoy commented Apr 26, 2019 •

edited

Add Scoped Notification Component #1785

Add Scoped Notification Component #1785

Conversation

aswinshenoy commented Mar 4, 2019 • edited

Additional description

Screenshots

CONTRIBUTOR checklist (do not remove)

REVIEWER checklist (do not remove)

Required only if there are markup / UX changes

interactivellama commented Mar 10, 2019

aswinshenoy commented Mar 18, 2019

interactivellama 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

interactivellama commented Apr 26, 2019

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

aswinshenoy left a comment • edited

Choose a reason for hiding this comment

interactivellama commented Apr 26, 2019

aswinshenoy commented Apr 26, 2019 • edited

aswinshenoy commented Mar 4, 2019 •

edited

aswinshenoy left a comment •

edited

aswinshenoy commented Apr 26, 2019 •

edited