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

Feature/cms 660 show read-only settings when allowAdminChanges is disabled #16265

Conversation

i-just
Copy link
Contributor

@i-just i-just commented Dec 3, 2024

Description

Shows read-only settings when allowAdminChanges is disabled.

  • switched .revision-notice to a generic .content-notice class so that the styles can be reused for other purposes too
  • when admin changes are disallowed, the “Settings” main nav item and all the subpages (except for the plugin settings) are shown, with all the fields and forms being disabled and with a notice at the top of every page
  • plugins can opt into having their settings shown when allowAdminChanges is disabled; to do so, they need to ensure none of the settings are editable in such case and set public bool $hasReadOnlyCpSettings to true

Further info are in Brandon's comment below.

Screenshot 2024-12-06 at 07 47 21

Related issues

cms-660

Copy link

linear bot commented Dec 3, 2024

@Mosnar
Copy link
Contributor

Mosnar commented Dec 4, 2024

Love this! It might be worth adding a "Learn More" button to a KB article explaining why settings can't/shouldn't be changed in production. Newer Craft developers or over-confident clients might read the current disclaimer as instructions to simply turn on "allowAdminChanges" and potentially get themselves in a mess.

@i-just i-just marked this pull request as ready for review December 4, 2024 14:36
Copy link
Contributor

@gcamacho079 gcamacho079 left a comment

Choose a reason for hiding this comment

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

@i-just awesome! I pinged you on the Notion page for this feature as I had some follow-up questions.

@@ -2917,6 +2930,7 @@ public static function layoutElementSelectorHtml(
'has-custom-width' => $element->hasCustomWidth(),
'has-settings' => $element->hasSettings(),
],
'disabled' => true,
Copy link
Member

Choose a reason for hiding this comment

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

@i-just What’s the reason for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was adding a disabled attribute to the fields inside the field layout designer’s .fld-field-library. But with the changes from #16374, it’s no longer needed (plus, it looks like I forgot to make it dynamic). I took that out.

@brandonkelly brandonkelly force-pushed the feature/cms-660-show-read-only-settings-when-allowadminchanges-false branch from bd582a5 to 3f3a6dd Compare January 7, 2025 23:53
@brandonkelly
Copy link
Member

brandonkelly commented Jan 8, 2025

I changed things up a bit so now all configurable components have a getReadOnlySettingsHtml() method, which by default will return the same thing as getSettingsHtml() except with all inputs disabled (similar to Field::getStaticHtml()).

Fields, filesystems, and mail transport adapters now override getReadOnlySettingsHtml() whenever possible, with the exception of BaseOptionsField and BaseRelationField (since in those cases, there’s no way to explicitly override getReadOnlySettingsHtml() without leaving 3rd party subclasses in the dust, so it’s better to wait until v6 before updating those).

Plugins similarly now have a getReadOnlySettingsResponse() method, and a $hasReadOnlyCpSettings property. craft\base\Plugin::getReadOnlySettingsResponse() will return $this->getSettingsResponse() with inputs in the settings HTML disabled (like ConfigurableComponent::getReadOnlySettingsHtml() and Field::getStaticHtml()), and its init() method will automatically set $this->hasReadOnlyCpSettings = true if getSettingsResponse() hasn’t been overridden.

The point of these changes is to limit the need to check allowAdminChanges, in case there’s a reason to render any of these settings without taking allowAdminChanges into effect. For example, a form plugin might have a need to render a Plain Text field’s settings even when allowAdminChanges is disabled (because maybe the field isn’t getting saved to the project config), and should still be able to call $field->getSettingsHtml() and get the normal non-disabled response.

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

Successfully merging this pull request may close these issues.

4 participants