-
Notifications
You must be signed in to change notification settings - Fork 434
Publisher: Adds basic component #1913
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
Conversation
Publisher component was missing in DSR. This commit roughly implements the component. More props are yet to be added and tests are to be added too.
@interactivellama @futuremint This is now ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just props proposal feedback right now.
/** | ||
* This label appears above the text area | ||
*/ | ||
label: PropTypes.string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the SLDS site, there is no visible label. Therefore this should be assistiveText.label
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/** | ||
* This label appears above the text area | ||
*/ | ||
label: PropTypes.string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be a labels.comment
for the comment text button, so that it can be changed. Also any "English text" in this component for instance add user title
should be here. the Share button, etc.
* Unique ID for the component to enable ARIA functioning and keyboard interactions | ||
*/ | ||
id: PropTypes.string, | ||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recommend an avatar
prop for use by the comment variant. This would allow reuse and customization of the Avatar component to the consuming developers needs including clicking the Avatar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will add an avatar
prop and update the PR soon.
/** | ||
* Function callback on click of submit button | ||
*/ | ||
onSubmit: PropTypes.func.isRequired, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this double function for share and comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's for both of those.
This issue has been automatically marked as stale, because it has not had recent activity. It will be closed if no further activity occurs. Maintainers are responsible for tech debt and project health. This is most likely a new components or component feature request. Please submit a pull request for or request feedback on this feature. Thank you. |
Publisher component was missing in DSR. This PR is focussed at the implementation of the component in DSR.
Fixes #1910
Additional description
CONTRIBUTOR checklist (do not remove)
Please complete for every pull request
npm run lint:fix
has been run and linting passes.components/component-docs.json
CI tests pass (npm test
).REVIEWER checklist (do not remove)
components/component-docs.json
tests.Required only if there are markup / UX changes
last-slds-markup-review
inpackage.json
and push.last-accessibility-review
, topackage.json
and push.npm run local-update
within locally cloned site repo to confirm the site will function correctly at the next release.