-
Notifications
You must be signed in to change notification settings - Fork 18
IBX-10414: Drop save action from forms #1666
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
Conversation
1904f81
to
a04dfc4
Compare
@@ -256,14 +241,9 @@ | |||
<target state="new">Name</target> | |||
<note>key: role_update.name</note> | |||
</trans-unit> | |||
<trans-unit id="453d030fbc11c10c842807ff213039adfffbe051" resname="role_update.save"> | |||
<trans-unit id="004e2e90e1b877d6a3c2def0a6f50ad62ef5f019" resname="role_update.save_and_close"> |
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.
The key here is "role_update.save_and_close", but the translation is "Save". Is it intended?
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.
Yes in all cases - the button changed label but the functionality did not change - I didnt want to mess up with someone translation as we could end with translated Save and close
but what we want is just Save
there.
src/bundle/Resources/translations/ibexa_content_forms_role.en.xliff
Outdated
Show resolved
Hide resolved
src/lib/Form/Type/ContentTypeGroup/ContentTypeGroupCreateType.php
Outdated
Show resolved
Hide resolved
@@ -95,8 +72,7 @@ public function createStructure(array $options): ItemInterface | |||
public static function getTranslationMessages(): array | |||
{ | |||
return [ | |||
(new Message(self::ITEM__SAVE, 'ibexa_menu'))->setDesc('Save'), | |||
(new Message(self::ITEM__SAVE_AND_CLOSE, 'ibexa_menu'))->setDesc('Save and close'), | |||
(new Message(self::ITEM__SAVE_AND_CLOSE, 'ibexa_menu'))->setDesc('Save'), |
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.
(new Message(self::ITEM__SAVE_AND_CLOSE, 'ibexa_menu'))->setDesc('Save'), | |
(new Message(self::ITEM__SAVE, 'ibexa_menu'))->setDesc('Save'), |
?
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.
Those are buttons IDs, that would change id associated with current behaviour, ergo could introuduce some sort of bc breaks.
* @throws \InvalidArgumentException | ||
* @throws ApiExceptions\BadStateException | ||
* @throws \InvalidArgumentException | ||
*/ | ||
public function createStructure(array $options): ItemInterface | ||
{ | ||
$saveId = $options['save_id']; | ||
$saveAncCloseId = $options['save_and_close_id']; | ||
$saveAndCloseId = $options['save_and_close_id']; |
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.
Should it be save_and_close_id
as the button is about creating?
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.
they are all named like this, for update as well, sometimes they are named save_and_edit
, I really do not want to change all of them now. Even more that technicaly is BC break.
@mikadamczyk |
84c281f
to
498123d
Compare
|
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.
QA Approved on 5.0.x-dev
Related PRs:
Description:
For QA:
List of affected create/update forms:
Documentation: