[PAGES-118] Provide visual clues for possible URI2Repo and virtual URI mapping conflicts Created: 16/Jan/17 Updated: 24/Jan/21 Resolved: 31/Jan/17 |
|
| Status: | Closed |
| Project: | Magnolia pages module |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 5.5.2 |
| Type: | Improvement | Priority: | Neutral |
| Reporter: | Jan Haderka | Assignee: | Jan Haderka |
| Resolution: | Fixed | Votes: | 1 |
| Labels: | None | ||
| Remaining Estimate: | 1.5h | ||
| Time Spent: | 4.5h | ||
| Original Estimate: | 0.75d | ||
| Attachments: |
|
||||||||||||||||||||
| 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
|
||||||||||||||||||||
| Documentation update required: |
Yes
|
||||||||||||||||||||
| Date of First Response: | |||||||||||||||||||||
| Sprint: | Kromeriz 81 | ||||||||||||||||||||
| Story Points: | 2 | ||||||||||||||||||||
| Description |
|
Some times editors would name a page in a way that conflicts with existing mapping for repository or with virtual URI mapping. Sometimes newly created mappings inadvertly hide already existing pages. Troubleshooting such situations is usually difficult as there's no clue provided and as a result given page generates just 404 error. |
| Comments |
| Comment by Christopher Zimmermann [ 17/Jan/17 ] |
|
This would be useful. |
| Comment by Jan Haderka [ 17/Jan/17 ] |
|
czimmermann This issue solves actually the opposite so I don't think there is a conflict. Anyway, the situation that you describe would be quite hard to "model" as there's no request parameters when browsing workbench so you would need to have pool of allowed values or some kind of parameter resolver and you would need a means to push that into VirtualURIManager - currently, it accepts just uri and resolves anything else from AggregationState so for example tours virtual uri mapping would never match. Same for the site specific uri mappings. But those are technical details. One thing I would argue for, would be other location for such warning than the status column, but admittedly, current one was easiest to implement and is used already to indicate other kind of statuses (e.g. readOnly-ness of the content) so it felt like natural place to add the warning to. |
| Comment by Christopher Zimmermann [ 18/Jan/17 ] |
|
After further consideration, I agree there is no conflict. |
| Comment by Anja von Gunten [ 18/Jan/17 ] |
|
had I have discussed this with Andreas. I would place the warning icon inside the column „page“ because the URI error deals with the page itself and not specifically with the (publication) status. Solution
Creating a new column is putting too much weight on this issue so thats why i favor option 2. I’m not sure if this is a warning or error indication so i visualized both. We could additionally add a background color to make it even clearer especially if the table is not fully visible on the screen. Additional
|
| Comment by Roman Kovařík [ 18/Jan/17 ] |
|
I'd not place it in the name column, it's already overcrowded (I mean the code, not the visual result) with icons for personalized pages. |
| Comment by Jan Haderka [ 18/Jan/17 ] |
|
avongunten thanks, I'll see what can be done in that regard |
| Comment by Roman Kovařík [ 18/Jan/17 ] |
|
^^Updated the comment. On the other hand I already made a generic wrapper while doing the POC for personalized assets... |
| Comment by Jan Haderka [ 18/Jan/17 ] |
|
So really, you were just looking for opportunity to mention it, right? Maybe you want to include this wrapper in the PR already or does it go directly in MGNLUI? |
| Comment by Jan Haderka [ 23/Jan/17 ] |
|
avongunten Except for popup text not being in green and columns with warning/error not having orange/red background it's done. Is this acceptable or not? ( BTW do we even have anything for choosing color of the popup text and for background color of whole row? apchelintcev mgeljic ) |
| Comment by Mikaël Geljić [ 23/Jan/17 ] |
|
Yep, column formatter/generator only gives you control over the text content within the cell, which can be html, but no control over style names generated for that cell/row; Vaadin Table gives you the CellStyleGenerator (which can add a style to the whole row when propertyId is null), while Vaadin Grid gives you an explicit RowStyleGenerator. Re: tooltip, I'm guessing you did it with plain title attribute; otherwise, in the Vaadin way it would display green, i.e. via setting the ItemDescriptionGenerator. In both cases, something you can only do on the component itself—that is if pages app had its own specific list/tree presenter/view indeed. |
| Comment by Anja von Gunten [ 24/Jan/17 ] |
|
The green tooltip is for example implemented in the status bar at the bottom of the page editor. From UI perspective it would be preferred to follow one unified style for same patterns. Not sure about the ongoing technical discussion: Is there a technical obstacle not to do it the same way? |
| Comment by Jan Haderka [ 24/Jan/17 ] |
|
avongunten yes, it's technically more complicated but not unsurmountable. As Mika correctly guessed implementation currently uses same mechanism as other column formatters which is why it's grey. If you move mouse over activation status or over date columns you get grey warnings too. If we change it, I would suggest to change layout of ALL of those tips by changing the style rather then by changing implementation for just this one column. BTW, you didn't mention the part about background color in the columns. I would propose to leave that out until we reimplement workbench using grid. Is that acceptable or not? |
| Comment by Anja von Gunten [ 24/Jan/17 ] |
|
Agree, that it would be better to have one style for all tooltips. Just having it for one column doesn’t make a difference. So preferably change all or (if too much time & effort which is up to you to decide) then just leave it as it is in grey. Concerning the bg color: fine to wait until that reimplementation. Most important to me: 1. move the icon to the page column & align them vertically to the right side, 2. use the correct icons as shown in the mockup (which according to Andreas have been agreed on previously). Does that work for you? |
| Comment by Jan Haderka [ 24/Jan/17 ] |
| Comment by Jan Haderka [ 24/Jan/17 ] |
|
re all green tooltips, i'll see if i can track css change for that. |
| Comment by Mikaël Geljić [ 25/Jan/17 ] |
|
FYI tooltips set via title attribute are rendered by the browser and cannot be styled. |
| Comment by Jan Haderka [ 25/Jan/17 ] |
|
Ah, that answers that. Thanks mgeljic. avongunten so no way to change them all. Ok to leave it grey? |
| Comment by Anja von Gunten [ 25/Jan/17 ] |
|
Ok… but i want more snowflakes |
| Comment by Mikaël Geljić [ 27/Feb/17 ] |
|
mdrapela Good to know, thanks. For future reference, the icon class is not the place for specific position info (can be used anywhere else). |