[PAGES-61] DefaultValue does not work for page properties Created: 08/Mar/16 Updated: 26/Mar/20 Resolved: 05/May/16 |
|
| Status: | Closed |
| Project: | Magnolia pages module |
| Component/s: | None |
| Affects Version/s: | 5.4 |
| Fix Version/s: | 5.4.6 |
| Type: | Improvement | Priority: | Neutral |
| Reporter: | Roman Kovařík | Assignee: | Roman Kovařík |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | ux | ||
| Remaining Estimate: | 0d | ||
| Time Spent: | 20m | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Template: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Acceptance criteria: |
Empty
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Release notes required: |
Yes
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Date of First Response: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Sprint: | Kromeriz 34, Kromeriz 42 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Story Points: | 3 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Description |
|
A "defaultValue" property does not work in Page Dialogs: a field is not displayed with the same value assigned to property 'defaultValue' into the correspondent dialog field. |
| Comments |
| Comment by Roman Kovařík [ 08/Mar/16 ] |
|
The problem is that the page is created via Add page action. If you edit the page properties, the default values are not taken into account since the item is not an instance of JcrNewItemAdapter anymore so the default values are not used. |
| Comment by Mikaël Geljić [ 14/Mar/16 ] |
|
This should most likely have gone through ux approval upfront; in the past we took the stance several times to acknowledge the issue and leave it aside (while auto-generation was a solution for areas). Not saying this is necessarily wrong, but it needs to be looked at carefully. Upon discussion, we could see at least two angles to address this issue: 1. Without impact on user interaction, by means of dialog-based auto-generation
2. By chaining dialogs for the "Add page" action, like you did
I think both are fine, so we can generally proceed with 2. but we need to make a few amendments then:
Did I miss anything? or more ideas/follow-up for simplification of page models? |
| Comment by Andreas Weder [ 14/Mar/16 ] |
|
Hi guys, I've attached a wireframe and have given this more thought. mgeljic, if we actually label that button "Add" instead of "Choose" (it's not just about choosing a template anyway), then we could actually mostly go with the existing solution, don't you think? So what I would change:
Optionally, I would also:
To clarify even more that clicking on "Add" does indeed add the page, we could (see second wireframe):
I'm sure that many users will welcome getting the page properties right away, as getting to them right after adding the page is a bit cumbersome (click on "edit page" first, then "edit page properties"). Should we find that not be the case, this solution will indeed add one more click to get going on a page. I'll add a follow-up issue asking for a mechanism more like solution 1 you sketched, mgeljic, to at least get this researched, to also tackle other "defaultValue" cases. |
| Comment by Mikaël Geljić [ 17/Mar/16 ] |
|
weder, can't say I'm too fond of it:
This fuss about default values is old enough, and didn't become critical all of a sudden, so we have choice |
| Comment by Roman Kovařík [ 21/Mar/16 ] |
|
This ticket is about page properties. The default values for components work without problems when adding a component by a dialog. Default values for autogenerated components/areas are different topic. You can provide all required values for an auto generated component https://documentation.magnolia-cms.com/display/DOCS/Component+autogeneration#Componentautogeneration-Autogenerationbehavior or you can create your own CopyGenerator to meet your needs (and e.g. hijack the dialog default values)... I don't know where did the myth about problems with areas/components default values come from but I see problems only with page properties |
| Comment by Mikaël Geljić [ 21/Mar/16 ] |
Did I mention any? |
| Comment by Andreas Weder [ 01/Apr/16 ] |
|
I've discussed this again with mgeljic. I have switched views, since I've learned that the proposed solution you've developed, rkovarik, actually explicitly saves a newly created page before showing the page properties. I was under the impression that we have less control over when we save the page. Under this changed conditions, I'm more in favor of mgeljic's proposal: I'd only create the new page once the user presses "save changes" on the second dialog, also because it aligns this behavior aligned with the one when adding a new component. Here's the flow I'd like to see realized:
2) We show the "page properties" dialog
3) If I press "save changes", the page is created. If I press "cancel", the page is not created. For now, we ignore cases where no page properties dialog has been configured. Such technically possible cases would ask for a different button label instead of "next". Apart from these changes related to the flow and behavior, I'd also like to make additional changes to both dialogs, but I've factored them out into a follow-up issue |
| Comment by Roman Kovařík [ 11/Apr/16 ] |
|
Reopened since it was reverted until |
| Comment by Roman Kovařík [ 21/Apr/16 ] |
|
Blocked till M5.4.6 is released. |
| Comment by Philip Mundt [ 22/Apr/16 ] |
|
This solution is flawed: it will not fire ContentChangedEvent s for the newly created page resulting in
Until further notice I will revert the efforts put into the UI tests (I might integrate a similar refactoring of the #addNewPage(...) method though; I had done something similar previously which was not integrated). |
| Comment by Roman Kovařík [ 25/Apr/16 ] |
|
pmundt Do you have the current branch? It seems that focus/selection works as expected. |
| Comment by Philip Mundt [ 26/Apr/16 ] |
|
rkovarik yes I can verify, it works on the respective branch. What I had done last week was reverting the reverts which didn't do the trick IIRC. |
| Comment by Philip Mundt [ 27/Apr/16 ] |
|
Unfortunately, I still believe there a two (minor) issues with this PR:
|
| Comment by Roman Kovařík [ 27/Apr/16 ] |
|
| Comment by Milan Divilek [ 29/Apr/16 ] |
|
Reopen: Following warn is logged after clicking on next button in add page dialog when adding page under the root.
2016-04-29 09:43:21,969 WARN agnolia.ui.workbench.tree.HierarchicalJcrContainer: Cannot determine parent for itemId: info.magnolia.ui.vaadin.integration.jcr.JcrNewNodeItemId@303012d2: javax.jcr.ItemNotFoundException: Root node doesn't have a parent
|
| Comment by Antonín Juran [ 05/May/16 ] |
|
Isn't updated configuration for create page dialog. |
| Comment by Roman Kovařík [ 30/May/16 ] |
|
For release notes: |