Feature/entries with category features #11749

myleshyson · 2022-08-07T16:49:37Z

Description

This is a continuation of the work @timkelty started. Since the logic for the categories field already works so well, my goal here was to take the existing ancestor logic from CategoryInput and craft\fields\Categories and merge it into BaseElementSelectInput and craft\fields\BaseRelationField.

As a result of this, the categories field no longer needs its own CategorySelectInput.js module, since the BaseElementSelectInput.js module covers everything structure related.

There are some things I'm unsure about, namely being:

Branch limit and Limit in the base relation field settings. Branch limit effectively acts like limit for non structured elements, so not sure if limit should just be replaced with branch limit, or vice versa. For backwards compatibility sake maybe we can keep one of those settings and have it set the other on save?
I moved craft\controllers\CategoriesController::actionInputHtml to craft\controllers\ElementsController::inputHtml. Was torn between this and moving it to craft\controllers\StructuresController::actionInputHtml since this really is structure specific. However the StructuresController has a required elementId param for all requests currently which doesn't make sense for this specific route. What do ya'll think?
Kept the CategorySelectInput module in tact. Figured there might be people out there extending it. With this pull request though it's dead code.
Also not sure what the protocol is for contributing to the js modules since the build fiels are included in the repo. I ran npm run build before opening this but let me know if there's more I should do!

Would love feedback on this! All in all this pull request is more moving pre-existing code around than adding anything new.

Related issues

#9781
Initial Pull Request: #11088

…er::inputHtml to ElementsController::inputHtml. extract field element listings to their own twig files.

…guage from base relational field

myleshyson · 2022-08-08T13:21:11Z

I'm also considering adding a migration script, maybe some thing like craft utils/migrate-categories-to-sections. If ya'll think that's a good idea I can add it to this pull request or create a new one.

timkelty · 2022-08-08T13:22:19Z

Thanks @myleshyson! Will review ASAP

…e/entries-with-category-features

Did you mean this? npm run build # run the "build" package script To see a list of supported npm commands, run: npm help

timkelty · 2022-08-22T17:06:06Z

@myleshyson Finally getting around to reviewing this – could you resolve the conflict in src/web/assets/cp/src/js/BaseElementSelectInput.js that has diverged since you originally made this?

No need to worry about the other conflicts.

…ts on remove

myleshyson · 2022-08-23T14:12:50Z

@timkelty alright fixed that conflict. not sure what that bit of code removing hidden input names addresses, but I added it for structured elements as well so there's parody.

myleshyson · 2022-08-23T17:36:26Z

Ah ok so that was to address this #11789, which should now be addressed for structured elements as well after I resolved the merge conflict.

timkelty

@myleshyson One aspect that is currently broken is when you have multiple section sources selected.

I think the best approach would be enforce that relateAncestors is only possible when a single structure source is selected. On the settings page, that option would only display when that condition was met.

Similarly, I think the branchLimit option only becomes available under those same conditions.

src/fields/BaseRelationField.php

src/fields/Categories.php

timkelty · 2022-08-29T17:25:31Z

Branch limit and Limit in the base relation field settings. Branch limit effectively acts like limit for non structured elements, so not sure if limit should just be replaced with branch limit, or vice versa. For backwards compatibility sake maybe we can keep one of those settings and have it set the other on save?

Since they do pretty different things, I would say keep them separate, but when branchLimit is set, we always set limit to null (and the corresponding setting for limit would disappear)

timkelty · 2022-08-29T17:28:09Z

I moved craft\controllers\CategoriesController::actionInputHtml to craft\controllers\ElementsController::inputHtml. Was torn between this and moving it to craft\controllers\StructuresController::actionInputHtml since this really is structure specific. However the StructuresController has a required elementId param for all requests currently which doesn't make sense for this specific route. What do ya'll think?

I think where you have it makes sense.

Kept the CategorySelectInput module in tact. Figured there might be people out there extending it. With this pull request though it's dead code.

Yep, that's the right approach. Keep it in place but include an @deprecated comment.

Also not sure what the protocol is for contributing to the js modules since the build fiels are included in the repo. I ran npm run build before opening this but let me know if there's more I should do!

Nope, you nailed it!

timkelty · 2022-08-29T17:28:56Z

Great work, @myleshyson!
Some remaining questions and issues in the preceding comments. Holler if you have any questions.

myleshyson · 2022-08-29T20:36:59Z

Awesome thanks! I'll address these either tonight or tomorrow morning.

…t relational fields. Update logic in BaseRelational field to clear limits depending on whether relatAncestors is checked or not. some minor cleanup

timkelty · 2022-09-06T15:59:46Z

@myleshyson could you also fix the ECS and PHPstan errors?

timkelty · 2022-09-07T01:50:24Z

I think ideally, the instruction text is also dimmed when disabled (I realize that makes it harder, as it means using js to set a class on the parent):

timkelty · 2022-09-07T01:51:49Z

@myleshyson "Relate Ancestors" currently remains enabled when selecting a non-structure channel. It should only enabled when a single structure source is selected, right? Should probably also remain disabled if nothing is checked

timkelty · 2022-09-07T02:30:53Z

We should think about this scenario:

Existing field, w/o relateAncestors checked
Existing entry with a structural child selected
Field is edited, enabling relateAncestors

With the current implementation, when you load the entry, the single entry will be related, and the relations are not filled in on save. The ancestor selection is only enforced on selection.

I'm thinking we should enforce the setting on save as well, to support this scenario.

myleshyson · 2022-09-07T12:05:06Z

@myleshyson "Relate Ancestors" currently remains enabled when selecting a non-structure channel. It should only enabled when a single structure source is selected, right? Should probably also remain disabled if nothing is checked

I need to think some on how to implement. Right now the only thing passed are source keys to the twig template. If I instead pass the source arrays from within getInputSources, but that would definitely be a breaking change for some people who extend the BaseRelationField class. Maybe I'll just pass an extra template variable called strucutredSources or something like that to keep things backwards compatible.

timkelty · 2022-09-07T12:10:34Z

@myleshyson "Relate Ancestors" currently remains enabled when selecting a non-structure channel. It should only enabled when a single structure source is selected, right? Should probably also remain disabled if nothing is checked

I need to think some on how to implement. Right now the only thing passed are source keys to the twig template. If I instead pass the source arrays from within getInputSources, but that would definitely be a breaking change for some people who extend the BaseRelationField class. Maybe I'll just pass an extra template variable called strucutredSources or something like that to keep things backwards compatible.

Sounds good. If you'd rather focus on the other issue, we can shop around approaches for this too.
I feel like it might be a good idea to start by validating this on the backend (on field settings save) so it isn't even possible, then deal with the frontend?

timkelty · 2022-09-12T15:42:36Z

@myleshyson hey there! Just checking in where you are with the remaining things and what you still plan on working on. Asking because we're planning the coming 4.3 release, and I wanted to see if we should try and fit this in.

myleshyson · 2022-09-12T16:31:16Z

Hey @timkelty. The goal was to work on it over the weekend but a lot of stuff came up this weekend unfortunately. The plan is to work on this between today and tomorrow.

… ancestors field

myleshyson · 2022-09-12T23:30:28Z

@timkelty alright updated the branch. Instructions are disabled out. There's a validator for sources now when relateAncestors is checked.

I'm forcing the relateAncestors setting on element save now. Only downside is that afterElementSave always runs if relateAncestors is checked, where before it only ran if the field value was dirty. Given the above scenario where the settings changed, not sure as of now how to get around that. Might not be a big deal though.

src/controllers/ElementsController.php

…tion for clarity

timkelty · 2022-09-13T20:35:46Z

Hell of a job, @myleshyson!
Great work.

I'm going to change the target of this to my PR, merge it, so I can add changelog, etc – then I'll recommend it for 4.3 🥳

We will likely work on a CLI command to migrate an existing category to section – let me know if you have any thoughts/ideas there.

One note:
Guessing you know this, but ideally we'd know when you checked a source that wasn't structured, we would know in JS and be able to disable the input. It looked like that would probably be overly complicated to pull off, and I think the backend validation is what's most important anyway.

brandonkelly · 2022-11-22T19:50:09Z

This is now merged for Craft 4.4 🚀

The setting has been renamed to “Maintain hierarchy”, and it’s now hidden whenever a non-structured source is selected.

[ci skip]

brandonkelly · 2023-03-08T22:17:29Z

Craft 4.4 is out with that change!

timkelty and others added 5 commits February 22, 2022 09:11

Initial work

468486a

merge in tims initial work

e17daa4

begin merging in category markup into element selector field.

75bb0ca

move CategoryInput logic to BaseElementInput. Move CategoriesControll…

48868ae

…er::inputHtml to ElementsController::inputHtml. extract field element listings to their own twig files.

move branch limit next to relateAncestors field in settings

75c43f9

myleshyson requested review from benjamindavid, nfourtythree and a team as code owners August 7, 2022 16:49

a little cleanup. give branch limit a default value. remove entry lan…

3c0f8d0

…guage from base relational field

timkelty self-assigned this Aug 8, 2022

brandonkelly changed the base branch from develop to 4.3 August 8, 2022 14:24

brandonkelly self-requested a review as a code owner August 8, 2022 14:24

myleshyson added 2 commits August 15, 2022 16:11

Merge branch '4.3' of https://github.com/myleshyson/craft into featur…

a0005bf

…e/entries-with-category-features

merge in 4.3. re-run Unknown command: "build"

28aa6be

Did you mean this? npm run build # run the "build" package script To see a list of supported npm commands, run: npm help

brandonkelly force-pushed the 4.3 branch from 71e9c59 to 804f8e8 Compare August 17, 2022 18:12

fix conflict. also remove inputs from form data for structured elemen…

e7d7be6

…ts on remove

timkelty requested changes Aug 29, 2022

View reviewed changes

src/fields/BaseRelationField.php Outdated Show resolved Hide resolved

src/fields/BaseRelationField.php Outdated Show resolved Hide resolved

src/fields/Categories.php Show resolved Hide resolved

brandonkelly mentioned this pull request Aug 30, 2022

[4.x]: Cannot reorder selected categories from a category field #11828

Closed

myleshyson added 3 commits August 30, 2022 12:51

Add Craft.ElementFieldSettings module to handle view logic for elemen…

cc7cffa

…t relational fields. Update logic in BaseRelational field to clear limits depending on whether relatAncestors is checked or not. some minor cleanup

merge in 4.3 and fix conflict

1fe86b9

allow categories to act like entries

1a524c0

myleshyson added 6 commits September 12, 2022 16:11

address formatting errors. disable instructions when disabling relate…

1cc6fa1

… ancestors field

add sources validator for when relateAncestors is checked.

d4bcd3c

fix indentation

c9f4c65

force relation save when field relates ancestors

d17cc6f

sync with 4.3

1013dbc

updated disabled state for relateAncestors instructions

8228d6b

timkelty reviewed Sep 13, 2022

View reviewed changes

src/controllers/ElementsController.php Show resolved Hide resolved

myleshyson added 2 commits September 13, 2022 00:07

use translation function for model errors. rename structured input ac…

cb550ec

…tion for clarity

update source validation message

6da08a8

timkelty changed the base branch from 4.3 to feature/category-features-to-entries September 13, 2022 20:42

timkelty changed the base branch from feature/category-features-to-entries to 4.3 September 13, 2022 20:42

timkelty changed the base branch from 4.3 to feature/category-features-to-entries September 13, 2022 20:44

timkelty merged commit 6da08a8 into craftcms:feature/category-features-to-entries Sep 13, 2022

timkelty mentioned this pull request Sep 13, 2022

Add “Maintain Hierarchy” setting to Entries and Categories fields #11088

Merged

myleshyson deleted the feature/entries-with-category-features branch September 13, 2022 21:07

brandonkelly added a commit that referenced this pull request Feb 3, 2023

#11749 resolved #10560 as well

80061fd

[ci skip]

This was referenced Feb 18, 2023

Use DI for cookie config #12699

Closed

Activation validation #12712

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/entries with category features #11749

Feature/entries with category features #11749

myleshyson commented Aug 7, 2022 •

edited

Loading

myleshyson commented Aug 8, 2022

timkelty commented Aug 8, 2022

timkelty commented Aug 22, 2022 •

edited

Loading

myleshyson commented Aug 23, 2022

myleshyson commented Aug 23, 2022

timkelty left a comment •

edited

Loading

timkelty commented Aug 29, 2022

timkelty commented Aug 29, 2022

timkelty commented Aug 29, 2022

myleshyson commented Aug 29, 2022

timkelty commented Sep 6, 2022

timkelty commented Sep 7, 2022

timkelty commented Sep 7, 2022 •

edited

Loading

timkelty commented Sep 7, 2022

myleshyson commented Sep 7, 2022

timkelty commented Sep 7, 2022

timkelty commented Sep 12, 2022

myleshyson commented Sep 12, 2022

myleshyson commented Sep 12, 2022

timkelty commented Sep 13, 2022 •

edited

Loading

brandonkelly commented Nov 22, 2022

brandonkelly commented Mar 8, 2023

Feature/entries with category features #11749

Feature/entries with category features #11749

Conversation

myleshyson commented Aug 7, 2022 • edited Loading

Description

Related issues

myleshyson commented Aug 8, 2022

timkelty commented Aug 8, 2022

timkelty commented Aug 22, 2022 • edited Loading

myleshyson commented Aug 23, 2022

myleshyson commented Aug 23, 2022

timkelty left a comment • edited Loading

Choose a reason for hiding this comment

timkelty commented Aug 29, 2022

timkelty commented Aug 29, 2022

timkelty commented Aug 29, 2022

myleshyson commented Aug 29, 2022

timkelty commented Sep 6, 2022

timkelty commented Sep 7, 2022

timkelty commented Sep 7, 2022 • edited Loading

timkelty commented Sep 7, 2022

myleshyson commented Sep 7, 2022

timkelty commented Sep 7, 2022

timkelty commented Sep 12, 2022

myleshyson commented Sep 12, 2022

myleshyson commented Sep 12, 2022

timkelty commented Sep 13, 2022 • edited Loading

brandonkelly commented Nov 22, 2022

brandonkelly commented Mar 8, 2023

myleshyson commented Aug 7, 2022 •

edited

Loading

timkelty commented Aug 22, 2022 •

edited

Loading

timkelty left a comment •

edited

Loading

timkelty commented Sep 7, 2022 •

edited

Loading

timkelty commented Sep 13, 2022 •

edited

Loading