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 @wordpress/fields package #65230

Merged
merged 7 commits into from
Sep 11, 2024

Conversation

gigitux
Copy link
Contributor

@gigitux gigitux commented Sep 11, 2024

What?

This PR adds the @wordpress/fields package.

Why?

The @wordpress/fields package introduces a collection of data field controls designed specifically for WordPress UI and needs. For example, this package will include controls for:

These field controls provide streamlined and consistent functionality for managing essential WordPress elements.

Copy link

github-actions bot commented Sep 11, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: gigitux <[email protected]>
Co-authored-by: youknowriad <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.


export const privateApis = {};

lock( privateApis, {} );
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of locking the APIs, I think we should probably make this package a "bundled one" just like the "dataviews" package itself. I know there are two files (configs) to change to make a package a bundled package tools/webpack/packages.js and packages/dependency-extraction-webpack-plugin/lib/util.js

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed with 7e04069

@@ -0,0 +1,23 @@
# Fields

This package includes a set of field controls for the DataView library, designed for creating and managing data display elements within WordPress.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should hold more than just field controls. I believe the whole editor/src/dataviews existing folder belongs in this package.

  • Fields
  • Actions
  • Controls

For WP entities basically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I improved the package description with 43b8a93:

This package provides core elements for the DataView library, designed to simplify the creation and management of data display elements in WordPress.

Let me know if it can work for you!

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.

I think one of the initial follow-ups to this would be to try to move the fields and actions from packages/editor/src/dataviews/ to this package.

{ "path": "../warning" },
{ "path": "../url" },
{ "path": "../notices" },
{ "path": "../dataviews" }
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the list above match the dependencies used in the package (empty for now)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! Fixed with b00ec03


## Usage


Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add the README's API docs autogeneration markers (see other package README's)

Copy link
Contributor Author

@gigitux gigitux Sep 11, 2024

Choose a reason for hiding this comment

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

Fixed with 6e71a98 and e5fd361

@gigitux gigitux requested a review from gziolo as a code owner September 11, 2024 10:05
@gigitux
Copy link
Contributor Author

gigitux commented Sep 11, 2024

I think one of the initial follow-ups to this would be to try to move the fields and actions from packages/editor/src/dataviews/ to this package.

Yeah! I agree! I will take a look to the packages/editor/src/dataviews/ folder!

@gigitux gigitux requested a review from youknowriad September 11, 2024 10:09
@gziolo gziolo added the [Type] New API New API to be used by plugin developers or package users. label Sep 11, 2024
@youknowriad
Copy link
Contributor

I would be ok approving for the sake of iteration/smaller PRs but can we do at least one PR on top of it to see actual usage of this package in practice?

@gigitux
Copy link
Contributor Author

gigitux commented Sep 11, 2024

I would be ok approving for the sake of iteration/smaller PRs but can we do at least one PR on top of it to see actual usage of this package in practice?

It makes sense!

I rebased #65196 on top of this PR. Unfortunately, I can't change the base branch of the linked PR because I'm pushing on my own fork.

Here’s how the slug field control PR looks with this branch as the destination: gigitux#2

This is something that I did to test the benefit of this package. Now, I will follow your suggestion and I'm going move the fields and actions from packages/editor/src/dataviews/ to this package.

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 I see that there's a good plan already for the next steps.

@youknowriad youknowriad merged commit 0c0e605 into WordPress:trunk Sep 11, 2024
60 checks passed
@github-actions github-actions bot added this to the Gutenberg 19.3 milestone Sep 11, 2024
@gziolo
Copy link
Member

gziolo commented Sep 11, 2024

Do you plan to publish this package to npm in the current shape with the next Gutenberg plugin RC release next week?

I have one question, the description of the package is:

This package provides core elements for the DataView library, designed to simplify the creation and management of data display elements in WordPress.

Is it tied exclusively to the DataView library? The name @wordpress/fields is very generic, and the term "fields" is quite loaded in the WordPress space: "custom fields" (example article), Fields API project.

@gigitux
Copy link
Contributor Author

gigitux commented Sep 11, 2024

Do you plan to publish this package to npm in the current shape with the next Gutenberg plugin RC release next week?

I'm planning to add more code to this package in the next days. Currently, we don't expect that this package will be used by other consumers: I read this documentation and it looks like that there isn't a way to disable the publishing of the package.

Do you see any particular disadvantages to publishing the package?

Is it tied exclusively to the DataView library? The name @wordpress/fields is very generic, and the term "fields" is quite loaded in the WordPress space: "custom fields" (example article), Fields API project.

Yes! I’m still exploring this project, and I wasn’t aware of the Fields API project. After reading the introduction, it seems there is some overlap with the goals of data views, but still, we are in the exploration phase stage.

@gziolo
Copy link
Member

gziolo commented Sep 11, 2024

Do you see any particular disadvantages to publishing the package?

There is no value in publishing a package that isn't functional and can't be consumed. It's a one-line in package.json, like here:

@oandregal
Copy link
Member

oandregal commented Sep 12, 2024

Do you see any particular disadvantages to publishing the package?

We should not have packages that are unused by core in this repository. Until that happens, I'd rather have it private. Given this has no-reversible consequences (publish to npm) and it's time-sensitive, I've prepared #65269 to mark it as private as Grzegorz suggested.

@gigitux
Copy link
Contributor Author

gigitux commented Sep 12, 2024

Do you see any particular disadvantages to publishing the package?

We should not have packages that are unused by core in this repository. Until that happens, I'd rather have it private. Given this has no-reversible consequences (publish to npm) and it's time-sensitive, I've prepared #65269 to mark it as private as Grzegorz suggested.

Ok! Thanks for tacking care of it! 🙇

@fabiankaegy
Copy link
Member

fabiankaegy commented Oct 2, 2024

Hey @gigitux 👋

Would you be able to help write a dev note for this for the 6.7 release? We are planning to have this as part of a larger Miscellaneous Editor Updates note.

We are hoping to get all drafts in by October 13th to leave some time for reviews before the RC1.

All Dev Notes get tracked in #65784 so feel free to leave a note there or ping me directly :)

Please let us know if you can assist with that.

Thanks in advance :)

@fabiankaegy fabiankaegy added the Needs Dev Note Requires a developer note for a major WordPress release cycle label Oct 2, 2024
@gigitux
Copy link
Contributor Author

gigitux commented Oct 4, 2024

Thanks for the ping, @fabiankaegy! 🙇

I'm not sure that we should include this in the dev note. Currently, this package is still experimental, we are exploring APIs, and currently I don't think that this package could be useful outside of Gutenberg.

I'm not very expert with this process, so let me include @youknowriad and @oandregal. What do you think? I'm happy to write dev notes if you think that it is needed!

(I'm not sure if with this comment you are referring to this PR)

@fabiankaegy
Copy link
Member

@gigitux okay, if this package is not bundled with WordPress core as one of the enqueued scripts we don’t need a dev note :)

@youknowriad
Copy link
Contributor

The package is not public API yet, so no need for dev note for now indeed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Dev Note Requires a developer note for a major WordPress release cycle [Type] New API New API to be used by plugin developers or package users.
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

5 participants