- 🇺🇸United States alison
We've got both custom_body_class and node_type_class modules on a site we're onboarding, and things get a little hinky when you try to fill in one or the other on the content type settings form.
**Total speculation** BUT I wonder if this module was created from a copy of the node_type_class module, and some stuff got accidentally left behind, and/or some stuff didn't get "found-and-replaced" during initial module development?
node_type_class
is the name of the field on the content type settings form in the node_type_class module:
https://git.drupalcode.org/project/node_type_class/-/blob/e880d11d2f42bc...So, that needs to be changed (and an update hook added).
OR... hear me out... the per-content-type class setting could be removed from custom_body_class, and users could be advised to add node_type_class to get that functionality -- and custom_body_class could be used exclusively for per-node body classes. Ehh??
- 🇫🇮Finland heikkiy Oulu
There is an open merge request to fix the issue in issue https://www.drupal.org/project/custom_body_class/issues/3026806 → . It seems like that issue has been closed accidentally and cannot be reopened without the maintainer.
We have been using the patch for a long time without problems. It seems like it still is mergeable to the latest version: https://git.drupalcode.org/project/custom_body_class/-/merge_requests/2.
- First commit to issue fork.
- Assigned to AstonVictor
- Issue was unassigned.
- Status changed to Needs review
12 months ago 9:03am 12 December 2023 - 🇺🇦Ukraine AstonVictor
Created a new MR.
Replaced 'node_type_class' settings with 'custom_body_class' in order not to have a conflict with Node type class → module.
We should also do it because
getThirdPartySetting()
method uses the module name as a first parameter.
I also fixed validation functions for node/node_type forms. - Status changed to Needs work
12 months ago 9:04am 12 December 2023 - 🇫🇮Finland heikkiy Oulu
I checked the merge request and to me it seems like it has really many unrelated and outside of scope changes.
The original patch I mentioned in #8 only deals with the wrong dependency: https://www.drupal.org/files/issues/2019-01-18/remove-node-type-class-fe... →
The merge request here https://git.drupalcode.org/project/custom_body_class/-/merge_requests/5/... seems to do a lot more like move help texts around etc. Should these be handled in this issue because the issue title does not really contain it or would it make sense to only fix the original issue and create new issue(s) for the other fixes?
It would make it easier to review this PR.
- Status changed to Needs review
12 months ago 9:10am 12 December 2023 - 🇺🇦Ukraine AstonVictor
Created a new MR.
Replaced 'node_type_class' with 'custom_body_class' in order to avoid conflicts with Node type class → module.
We should also use the 'custom_body_class' ingetThirdPartySetting()
method because it uses the module name as the first parameter.I also fixed validation callbacks for node/node_type forms and fixed some cs issues.
Since we updated the storage for classes, changes should be added to a new version of the module or we should create an update function to move settings from node_type_class to custom_body_class.Let me know what you think about it. thanks.
- 🇫🇮Finland heikkiy Oulu
I think the validation and code quality issues should have a new ticket and create the fixes there. It will be easier to track where and why the changes happened because now the issue title No dependency on the Node type class module does not cover those changes.
- 🇺🇦Ukraine AstonVictor
Hi @HeikkiY,
the module should not have any dependencies. that's why I removed all code related to node_type_class.
- 🇫🇮Finland heikkiy Oulu
@astonvictor I don't feel comfortable marking this as reviewed, sorry. You are for example changing the whole module description in this MR which is definitely outside of this issues scope.
Maybe we need to wait for the maintainer to comment on this.
- 🇺🇦Ukraine AstonVictor
FYI made another MR as well that only contains changes to remove all code related to the 'node_type_class'.
- Status changed to RTBC
12 months ago 12:34pm 12 December 2023 - 🇫🇮Finland heikkiy Oulu
The second smaller MR looks good to me.
+1 for RTBC for those changes.
- 🇺🇦Ukraine AstonVictor
hi @Jitendra verma,
any updates? could you review and merge it?