-
Notifications
You must be signed in to change notification settings - Fork 820
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
Persist GridField readonly state, add view button #8535
Persist GridField readonly state, add view button #8535
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some minor points of feedback. Nice work.
src/Forms/GridField/GridField.php
Outdated
} | ||
} | ||
|
||
// As the edit button may have been removed, add a view button if it doesn't have one | ||
if ($copyConfig->getComponentsByType(GridFieldViewButton::class)->count() === 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better expressed as if (!$copyConfig->getComponentByType(...))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it mixed up with removeComponentsByType
which only has a multiple selector method, I've now changed it to the singular.
src/Forms/GridField/GridField.php
Outdated
@@ -1009,6 +1029,9 @@ public function gridFieldAlterAction($data, $form, HTTPRequest $request) | |||
} | |||
|
|||
if ($request->getHeader('X-Pjax') === 'CurrentField') { | |||
if ($this->getState(true)->Readonly) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
true
is the default value. This can just be getState()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
src/Forms/GridField/GridField.php
Outdated
public function setReadonly($readonly) | ||
{ | ||
parent::setReadonly($readonly); | ||
$this->getState()->Readonly($readonly); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering, only because this is so ugly and IDE unfriendly, is this functionally equivalent to just getState()->Readonly = $readonly
? I think it's just a question of whether it uses __call()
or __set()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Resolves #3357 (remaining ACs/bugs)