-
Notifications
You must be signed in to change notification settings - Fork 2k
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/republicize preview #12753
Add/republicize preview #12753
Conversation
aae24a6
to
77d8d8f
Compare
<Gridicon icon="cross" /> | ||
</button> | ||
</header> | ||
<SharingPreviewPane { ...previewProps } /> |
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.
Copying over notes from the other PR:
This is optional, but if we're concerned with pass through props, perhaps:
{ children }
So in the caller this looks like:
<SharingPreviewModal isVisible={ isVisible } onClose={ onClose }>
<SharingPreviewPane siteId={ siteId } postId={ postId } message={ message } />
</SharingPreviewModal>
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.
If we pull out the sharing specific props, this component becomes reusable if we need a dialog that looks like a web preview. (We'd need to change class names though and move this into the component folder.)
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.
yup, it's a good idea. I think we should move this component into its folder straight in this PR.
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 don't mind waiting for a follow up PR to do this change though
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'll land this now and do the follow up extraction in another PR.
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.
Tested this enabled/disabled 👍 Let's 🚢 this for now. Do you mind following up on making the modal a generic component in another PR after this lands?
For a first pass I think taking the first enabled account works. |
This adds a preview modal to publicize and updates share action button styles.
Action buttons
breakpoint > 480px
breakpoint < 480px
Modal
breakpoint > 660px
breakpoint < 660px
Note
At the moment we only have previews for Facebook and Twitter. #12644 adds the remaining available services.
Testing
ENABLE_FEATURES=publicize/preview make run