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.

Sign up for GitHub

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

Jump to bottom

Connnect gutenberg widgets screen to widget area endpoints #15074

Merged
merged 11 commits into from
May 22, 2019
Merged

Connnect gutenberg widgets screen to widget area endpoints #15074

merged 11 commits into from
May 22, 2019

Conversation

jorgefilipecosta
Copy link
Member

@jorgefilipecosta jorgefilipecosta commented Apr 19, 2019 •
edited
Loading

Description

Depends on: #15435

This PR adds a store to keep the state in edit-widgets module, and connects the edit widgets screen to the widget-area endpoints being implemented in #15015.

It allows us to have PoC of the widget screen using blocks, covering the retrieving of legacy widgets, showing them and migrating the structure to blocks.

Legacy widgets are not working as expected because we have a bug on the widgets screen. The attributes of server-side rendered blocks are not correctly parsed in this screen.
If we paste wp.blocks.parse('<!-- wp:latest-posts {"postsToShow":34,"displayPostDate":true} /-->'); on the console while the editor is open the attributes are correctly parsed on the gutenberg-widgets screen the attributes are not parsed.

youknowriad
youknowriad reviewed Apr 19, 2019
View reviewed changes
Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

Didn't take a deep look yet, but that's a great start. Let's not add too much features and try to ship it step by step.

youknowriad
youknowriad reviewed Apr 19, 2019
View reviewed changes
packages/edit-widgets/src/store/actions.js Outdated
STORE_KEY,
'getWidgetAreas'
);
for ( const widgetArea of widgetAreas ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes me wonder if we needed a "batched" save endpoint. I know there's no precedent in the REST API endpoints about this though.

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess most of the times the user will not change all the areas, and soon as we have some kind of "isDirty" dection we will only save the changed areas.

Copy link
Member

Choose a reason for hiding this comment

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

I was contemplating having a POST endpoint which can update multiple areas while writing #14812. Probably best to not get ahead of ourselves, though, as the design may yet change.

@jorgefilipecosta
Copy link
Member Author

Didn't take a deep look yet, but that's a great start. Let's not add too much features and try to ship it step by step.

Hi @youknowriad, I guess this is the simples version we can have with just reading and write features without complex logic like "isDirty" and undo. This PR looks bigger because it contains the CPT and the endpoints which are going to be merged in #15014 and #15015 respectively. With these changes merged this PR is going to look much smaller.

@jorgefilipecosta jorgefilipecosta force-pushed the add/wordpress-area-endpoint branch from 56f0948 to b84a588 Compare April 19, 2019 16:13
@jorgefilipecosta jorgefilipecosta force-pushed the try/connetc-gutenberg-widgets-screen-to-widget-area-endpoints branch from 21b434e to 627f0cd Compare April 19, 2019 16:16
@jorgefilipecosta jorgefilipecosta force-pushed the add/wordpress-area-endpoint branch 8 times, most recently from 347a161 to 6f67d60 Compare May 3, 2019 10:08
@jorgefilipecosta jorgefilipecosta force-pushed the try/connetc-gutenberg-widgets-screen-to-widget-area-endpoints branch from 627f0cd to 4d3f032 Compare May 3, 2019 10:51
@jorgefilipecosta jorgefilipecosta marked this pull request as ready for review May 3, 2019 10:56
@jorgefilipecosta jorgefilipecosta changed the base branch from add/wordpress-area-endpoint to master May 3, 2019 10:56
@jorgefilipecosta jorgefilipecosta mentioned this pull request May 3, 2019
Merged
@jorgefilipecosta jorgefilipecosta force-pushed the try/connetc-gutenberg-widgets-screen-to-widget-area-endpoints branch from 4d3f032 to f71ce9c Compare May 3, 2019 11:31
@jorgefilipecosta jorgefilipecosta changed the base branch from master to add/wordpress-area-endpoint May 3, 2019 14:56
jorgefilipecosta added a commit that referenced this pull request May 6, 2019
@jorgefilipecosta
Fix: Server side rendered blocks are not parsed on the widgets screen. (
a90752e 
#15414)

Server-side rendered blocks are not being correctly parsed on widgets screen making legacy widget blocks not load correctly on #15074.
The problem was that the server side block definitions were not being bootstrapped on the widgets page.
@jorgefilipecosta jorgefilipecosta force-pushed the add/wordpress-area-endpoint branch 3 times, most recently from ac4967d to 9b918ae Compare May 9, 2019 17:22
@jorgefilipecosta jorgefilipecosta changed the base branch from master to FET/controls-package May 21, 2019 18:00
@jorgefilipecosta jorgefilipecosta changed the base branch from FET/controls-package to master May 21, 2019 20:12
@jorgefilipecosta jorgefilipecosta force-pushed the try/connetc-gutenberg-widgets-screen-to-widget-area-endpoints branch from c9f9d2d to 56b91da Compare May 21, 2019 20:12
youknowriad
youknowriad approved these changes May 21, 2019
View reviewed changes
Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

Thanks for all the updates, I have a last small remark

packages/edit-widgets/src/components/widget-areas/index.js Outdated Show resolved Hide resolved
@jorgefilipecosta jorgefilipecosta changed the base branch from master to FET/controls-package May 21, 2019 22:29
@nerrad nerrad force-pushed the FET/controls-package branch 2 times, most recently from 5afd069 to 5dfee2c Compare May 22, 2019 12:39
nerrad and others added 10 commits May 22, 2019 14:09
@nerrad @jorgefilipecosta
Packages: Add new @wordpress/data-controls package. (#15435)
084e92b 
* Add new @wordpress/data-controls package

* include new package in main package.json

* Implement new package with @wordpress/editor store

- this might be extracted to its own pull but added initially for testing.

* docs tool generating this for some reason

* include new package with doc generation tool

* updated manifest from doc generation

* updated package-lock.json

* not sure why docs build tool is doing this.

* use ternary

* data-controls added to manifest by docs build tool

* update README for editor package

* updates to editor store actions tests

- to use new data-controls package

* add missing `@example` tag and regenerate docs

* add tests for package

* fix typo in changelog for package name

* fix typo for package name in readme

* more typos

* Use  lower version for initial release.

Co-Authored-By: Grzegorz (Greg) Ziółkowski <grzegorz@gziolo.pl>

* Add heading for API.

Co-Authored-By: Grzegorz (Greg) Ziółkowski <grzegorz@gziolo.pl>

* Update CHANGELOG.md version reference.

Follows repo conventions about using Master as the version heading for unreleased changes.

* add data-controls to webpack plugin config

* Revert "add data-controls to webpack plugin config"

This reverts commit 36a0b98.

* use named export for controls object

* update docs

* woop fix import on tests
@jorgefilipecosta
Connect gutenberg widget screen to widget-area endpoints.
2f6676c
@jorgefilipecosta
Use saveEntityRecord.
6efefc5
@jorgefilipecosta
Enhancements
eafd71c
@jorgefilipecosta
Add test cases
9dda0ec
@jorgefilipecosta
Extracted a WidgetAreas component.
0ae87e1
@jorgefilipecosta
Improve the setup action to avoid the need for a parameter.
c24c204
@jorgefilipecosta
update test case
241056a
@jorgefilipecosta
Refactor pass widget are by id, have a save action.
fd920f2
@jorgefilipecosta
id instead of widgetAreaId + lint fix
4f2c6cc
@jorgefilipecosta jorgefilipecosta force-pushed the try/connetc-gutenberg-widgets-screen-to-widget-area-endpoints branch from 44ac2f2 to 4f2c6cc Compare May 22, 2019 13:45
@jorgefilipecosta jorgefilipecosta merged this pull request into FET/controls-package May 22, 2019
@jorgefilipecosta jorgefilipecosta deleted the try/connetc-gutenberg-widgets-screen-to-widget-area-endpoints branch May 22, 2019 14:47
@jorgefilipecosta jorgefilipecosta restored the try/connetc-gutenberg-widgets-screen-to-widget-area-endpoints branch May 22, 2019 15:17
@jorgefilipecosta jorgefilipecosta mentioned this pull request May 22, 2019
Merged
@jorgefilipecosta jorgefilipecosta deleted the try/connetc-gutenberg-widgets-screen-to-widget-area-endpoints branch May 22, 2019 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@youknowriad youknowriad youknowriad approved these changes

@aduth aduth Awaiting requested review from aduth

@gziolo gziolo Awaiting requested review from gziolo

@nerrad nerrad Awaiting requested review from nerrad

@noisysocks noisysocks Awaiting requested review from noisysocks

@ajitbohra ajitbohra Awaiting requested review from ajitbohra

@chrisvanpatten chrisvanpatten Awaiting requested review from chrisvanpatten

@mkaz mkaz Awaiting requested review from mkaz

@oandregal oandregal Awaiting requested review from oandregal

@notnownikki notnownikki Awaiting requested review from notnownikki

@ntwb ntwb Awaiting requested review from ntwb

@talldan talldan Awaiting requested review from talldan

Assignees
No one assigned
Labels
[Feature] Widgets Screen The block-based screen that replaced widgets.php. [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jorgefilipecosta @youknowriad @noisysocks @nerrad