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

Unnecessary placement rules in OrchardCore.Contents #16028

Closed
giannik opened this issue May 10, 2024 · 3 comments · Fixed by #16029
Closed

Unnecessary placement rules in OrchardCore.Contents #16028

giannik opened this issue May 10, 2024 · 3 comments · Fixed by #16029
Labels

Comments

@giannik
Copy link
Contributor

giannik commented May 10, 2024

Currently in OrchardCore.Contents module there is a placement rule in placements. json as so :

 "Content_PublishButton": [
    {
      "place": "Actions:5" // Same as in the driver
    }
  ], 

Apart from that the placement is already set from the driver this prevents other modules to add there own placement rule for this shape unless they take a dependency on this module.

Also there is a placement rule :

  "Content_SaveButton": [
    {
      "place": "Actions:6" // Move the save button after the publish
    }
  ],

that is not used anywhere as there is no shape with this name.
I propose to remove these 2 rules.

@Piedone
Copy link
Member

Piedone commented May 13, 2024

To clarify, do you only suggest removing the two Placement.json rules, or do you think the dependency being needed is also a bug? I agree with the former, not the latter.

@giannik
Copy link
Contributor Author

giannik commented May 13, 2024

no , adding a dependency is not a bug. But my understanding is that the 2 placement rules serve no purpose so just remove them to avoid confusion and the need to add the dependency.

@Piedone Piedone changed the title Placement rule prevents overriding shape in other modules Unnecessary placement rules in OrchardCore.Contents May 13, 2024
@Piedone
Copy link
Member

Piedone commented May 13, 2024

OK, thanks for the clarification!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants