Docked composer #3062

2FALockedMeOutOfMyAcct · 2022-08-26T17:30:23Z

Additional description

Adds Docked Composer to DSR

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/.
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)

CircleCI 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.

welcome · 2022-08-26T17:30:26Z

Thanks for opening this pull request! 💯

This is a community-driven project, and we can't do it without your participation. Please check out our contributing guidelines and review the Contributor Checklist if you haven't already, to make sure everything is squared away. CircleCI will take about 10 minutes to run through the same items that are on the Contributor checklist with a pass/fail check below. Please fix any issues that cause CircleCI to fail or ask for clarification--we try, but sometimes the errors can be unclear.
A maintainer will try to respond within 7 days. If you haven’t heard anything by then, please bump this thread. To ensure codebase quality, large code line changes may take more than 2 weeks to review, but may take longer depending on the number of pull requests in the queue. Feel free to ask for a status update at any time--you won't be bothering anyone.
Once feedback has been given, please reply to the feedback giver once the feedback on been addressed, so that they can continue the review.
If you need a release while you are waiting for a code review, you can publish a built tag to your own fork. See directions in the release README.

meizibupt · 2022-08-26T18:56:16Z

components/docked-composer/index.jsx

+						<div className="slds-col_bump-left slds-shrink-none">
+							<Button
+								id={`${this.getId()}-minimize-button`}
+								title="Minimize"


should we localize titles?

meizibupt · 2022-08-26T18:58:30Z

components/docked-composer/index.jsx

+	 * Text to display in the header.
+	 * _Tested with snapshot testing._
+	 */
+	header: PropTypes.string,


should we allow user to add a node for header too? so they can change icon etc

meizibupt · 2022-08-26T18:59:33Z

components/docked-composer/index.jsx

+import Button from '../button';
+
+const propTypes = {
+	/**


we can also add a css class prop to allow overriding css

alexxpan · 2022-08-26T19:10:48Z

components/docked-composer/__examples__/base.jsx

+	static displayName = 'DockedComposerExample';
+
+	state = {
+		isOpen: true,


This naming convention is a bit confusing, do you think isExpanded and isOpen would be a bit less ambiguous?

…poser state, add class name parameter, add assitive text parameters, add story for minimized state

2FALockedMeOutOfMyAcct · 2022-08-29T19:16:22Z

Minimized and expanded states:

3 votes in favor of this behavior: @gfyhui , @alexxpan and Jonathon

alexzherdev · 2022-08-29T22:13:52Z

components/docked-composer/__tests__/docked-composer.browser-test.jsx

+import { mount } from 'enzyme';
+import IconSettings from '../../icon-settings';
+
+// Import your internal dependencies (for example):


These comments seem like they can be removed

interactivellama

First, this is a great pull request! I did not pull this down to manually test it, but did left a few minor request and referenced the overview doc. Thank you for this great contribution!

interactivellama · 2022-08-29T22:17:01Z

components/docked-composer/__examples__/minimized.jsx

+				}}
+				id="docked-composer"
+				events={{
+					onMinimize: this.handleMinimize,


I haven't dove deep into this yet, but these look like on request pre-state change actions.
https://github.com/salesforce/design-system-react/blob/master/docs/codebase-overview.md#prefix-onrequestaction-to-pre-state-change-callbacks-functions

interactivellama · 2022-08-29T22:22:16Z

components/docked-composer/index.jsx

+				<section
+					className={`slds-docked-composer slds-grid slds-grid_vertical ${sectionClassName}`}
+					role="dialog"
+					aria-labelledby="dialog-heading-id-1"


All ids should be prefixed by an id or a random id if not set by the use. You are doing this in other locations: this.generatedId

interactivellama · 2022-08-29T22:23:14Z

components/docked-composer/index.jsx

+							{this.props.isOpen ? (
+								<Button
+									id={`${this.getId()}-minimize-button`}
+									title={this.props.assistiveText?.minimizeButton || 'Minimize'}


I think was would be typically done by a default props object, so no one has to search render functions for text literals.

interactivellama · 2022-08-29T22:23:29Z

components/docked-composer/index.jsx

+									title={this.props.assistiveText?.minimizeButton || 'Minimize'}
+									assistiveText={{
+										icon:
+											this.props.assistiveText?.minimizeButton || 'Minimize',


interactivellama · 2022-08-29T22:24:31Z

components/docked-composer/index.jsx

+							) : (
+								<Button
+									id={`${this.getId()}-expand-button`}
+									title={this.props.assistiveText?.expandButton || 'Expand'}


interactivellama · 2022-08-29T22:24:36Z

components/docked-composer/index.jsx

+									id={`${this.getId()}-expand-button`}
+									title={this.props.assistiveText?.expandButton || 'Expand'}
+									assistiveText={{
+										icon: this.props.assistiveText?.expandButton || 'Expand',


…enerated id for all components, use default props

interactivellama · 2022-08-30T23:05:01Z

@alexzherdev Merge when you ready to.

alexzherdev · 2022-08-30T23:22:11Z

Not super important for this simple component, but I wonder if we should encourage new components to be written as functions. At this point I don't think there are any pre-16.8 consumers?

welcome · 2022-08-30T23:24:40Z

Congrats on merging your first pull request to Design System React! 🎉
If you have a moment, please fill out this five question survey, we would appreciate it.
On behalf of Salesforce's customers, partners, product specialists and employees, we would like offer sincere thanks and appreciation for helping make our user experience better. We look forward to working with you more in the future.
This definitely calls for a high five!

interactivellama · 2022-08-31T16:29:08Z

@alexzherdev I'm cool with that. Please update the docs when convenient. I believe DataTable has hooks in it.

interactivellama · 2022-08-31T16:30:23Z

Also, love the account name: @2FALockedMeOutOfMyAcct

2FALockedMeOutOfMyAcct · 2022-08-31T21:14:49Z

Thanks for the reviews and merging. When will the next DSR release be @interactivellama @alexzherdev ?

alexzherdev · 2022-09-01T22:16:28Z

@interactivellama is the automated release build still broken?

interactivellama · 2022-09-01T22:23:01Z

Yes.

I could use some help in triaging the authentication, since the security protections went up.

interactivellama · 2022-09-07T16:28:56Z

@2FALockedMeOutOfMyAcct Please open a release PR and I will pull down and push up a tag.

alexzherdev · 2022-09-08T19:10:53Z

@interactivellama I opened #3063 if that's what you meant?

interactivellama · 2022-09-08T19:57:23Z

I'll try to get this out by End of week.

interactivellama · 2022-09-09T21:11:00Z

https://github.com/salesforce/design-system-react/releases/tag/v0.10.49

interactivellama · 2022-09-09T23:11:21Z

There are additional issues with the NPM publishing process. I'm in talks with Jim in OSS group on getting different permissions set up. The tag is present in Github but we do not have a NPM package up today.

gfyhui added 6 commits August 25, 2022 16:47

docked composer is working

7cd5eaf

test commit

3cffd80

add snapshots

d62f0a6

cleanup code, add test

98fbeca

add component docs

c63574a

fix indentation

0d80e00

salesforce-cla bot added the cla:signed label Aug 26, 2022

gfyhui added 2 commits August 26, 2022 10:32

remove unused imports

031eb29

add body prop

d0fcc13

meizibupt reviewed Aug 26, 2022

View reviewed changes

alexxpan reviewed Aug 26, 2022

View reviewed changes

gfyhui added 2 commits August 29, 2022 12:03

remove expand button, show minimize or expand button depending on com…

70bc632

…poser state, add class name parameter, add assitive text parameters, add story for minimized state

update props

694c32b

meizibupt previously approved these changes Aug 29, 2022

View reviewed changes

alexxpan previously approved these changes Aug 29, 2022

View reviewed changes

fix browser test

b970427

2FALockedMeOutOfMyAcct dismissed stale reviews from alexxpan and meizibupt via b970427 August 29, 2022 21:21

meizibupt previously approved these changes Aug 29, 2022

View reviewed changes

garygong previously approved these changes Aug 29, 2022

View reviewed changes

alexzherdev requested a review from interactivellama August 29, 2022 22:05

alexzherdev reviewed Aug 29, 2022

View reviewed changes

interactivellama requested changes Aug 29, 2022

View reviewed changes

remove boilerplate comments, rename callbacks to be onRequest*, use g…

50d90fe

…enerated id for all components, use default props

2FALockedMeOutOfMyAcct dismissed stale reviews from garygong and meizibupt via 50d90fe August 30, 2022 21:21

interactivellama approved these changes Aug 30, 2022

View reviewed changes

alexzherdev merged commit 40e14de into salesforce:master Aug 30, 2022

2FALockedMeOutOfMyAcct deleted the docked-composer branch August 31, 2022 00:02

Docked composer #3062

Docked composer #3062

Uh oh!

Conversation

2FALockedMeOutOfMyAcct commented Aug 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Additional description

CONTRIBUTOR checklist (do not remove)

REVIEWER checklist (do not remove)

Required only if there are markup / UX changes

Uh oh!

welcome bot commented Aug 26, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

meizibupt Aug 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

2FALockedMeOutOfMyAcct commented Aug 29, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

interactivellama left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

interactivellama Aug 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

interactivellama commented Aug 30, 2022

Uh oh!

alexzherdev commented Aug 30, 2022

Uh oh!

welcome bot commented Aug 30, 2022

Uh oh!

interactivellama commented Aug 31, 2022

Uh oh!

interactivellama commented Aug 31, 2022

Uh oh!

2FALockedMeOutOfMyAcct commented Aug 31, 2022

Uh oh!

alexzherdev commented Sep 1, 2022

Uh oh!

interactivellama commented Sep 1, 2022

Uh oh!

interactivellama commented Sep 7, 2022

Uh oh!

alexzherdev commented Sep 8, 2022

Uh oh!

interactivellama commented Sep 8, 2022

Uh oh!

interactivellama commented Sep 9, 2022

Uh oh!

interactivellama commented Sep 9, 2022

Uh oh!

Uh oh!

2FALockedMeOutOfMyAcct commented Aug 26, 2022 •

edited

Loading

meizibupt Aug 26, 2022 •

edited

Loading

interactivellama Aug 29, 2022 •

edited

Loading