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: className prop support to server side render. #13568

Merged
merged 2 commits into from
Jan 31, 2019

Conversation

jorgefilipecosta
Copy link
Member

@jorgefilipecosta jorgefilipecosta commented Jan 29, 2019

Description

Required by: #13511

In #13511 a need appeared where it was necessary to add some styles to the wrapper container of a ServerSideRender component.
In order to do that the easy way is to add className support for ServerSideRender.

How has this been tested?

I replaced the ServerSideRender usage on packages/block-library/src/archives/edit.js with <ServerSideRender className="my-random-class" block="core/archives" attributes={ attributes } /> .
I added an archives block I inspected the dom and I checked the class "my-random-class" was applied.

@jorgefilipecosta jorgefilipecosta added [Type] Enhancement A suggestion for improvement. [Package] Components /packages/components labels Jan 29, 2019
@gziolo gziolo added this to the 5.0 (Gutenberg) milestone Jan 30, 2019
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

I haven't tested but code-wise it looks very good. Can you also update docs to mention className in the list of supported props?

@jorgefilipecosta jorgefilipecosta force-pushed the add/class-prop-to-server-side-render branch from c362d0b to a044b48 Compare January 30, 2019 15:22
@jorgefilipecosta jorgefilipecosta force-pushed the add/class-prop-to-server-side-render branch from a044b48 to db5d777 Compare January 30, 2019 15:24
@jorgefilipecosta
Copy link
Member Author

Hi @gziolo,

I haven't tested but code-wise it looks very good. Can you also update docs to mention className in the list of supported props?

Docs were added.

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

I added a few tiny tweaks to the docs. Code wise everything looks great and tests well.

@jorgefilipecosta jorgefilipecosta merged commit 7fd6316 into master Jan 31, 2019
@jorgefilipecosta jorgefilipecosta deleted the add/class-prop-to-server-side-render branch January 31, 2019 12:31
@jorgefilipecosta
Copy link
Member Author

Thank you for the review and tweaks @gziolo!

daniloercoli added a commit that referenced this pull request Feb 1, 2019
…rnmobile/372-use-RichText-on-Title-block

* 'master' of https://github.com/WordPress/gutenberg:
  Try alternate list item jump fix. (#12941)
  Mobile bottom sheet component (#13612)
  Remove unintentional right-margin on last odd-item. (#12199)
  Introduce left and right float alignment options to latest posts block (#8814)
  Fix Google Docs table paste (#13543)
  Increase bottom padding on gallery image caption (#13623)
  Fix the editor save keyboard shortcut not working in code editor view (#13159)
  Plugin: Deprecate gutenberg_add_admin_body_class (#13572)
  Rnmobile/upload media failed state (#13615)
  Make clickOnMoreMenuItem not dependent on aria labels (#13166)
  Add: className prop support to server side render. (#13568)
  Fix: Categories Block: hierarchical Dropdown (#13567)
  Docs: Add clarification about git workflow (#13534)
  Plugin: Remove `user_can_richedit` filtering (#13608)
  eslint-plugin: Add rule `no-unused-vars-before-return` (#12828)
  Image settings button (#13597)
  Fixed wording for the color picker saturation (#13479)

# Conflicts:
#	packages/block-library/src/image/edit.native.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants