[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: PNG File 2017-02-27_triangleNotRenderedFirefox.png     PNG File Display warning.png     PNG File Screen Shot 2017-01-16 at 14.26.45.png     PNG File Screen Shot 2017-01-16 at 14.26.52.png     PNG File Screen Shot 2017-01-24 at 17.02.29.png     PNG File Tooltip warning.png    
Issue Links:
Relates
relates to PAGES-145 Conflicting URI mapping icons are mis... Closed
relates to PAGES-139 Check conflicting virtual URIs agains... Closed
relation
is related to CNTCTSAPP-98 Unable to edit a page whose name conf... Closed
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.
Aim is to provide clue for the editors in status column in pages app that such conflict might exist when we can detect it.
Possible solution shown in attached screenshots.



 Comments   
Comment by Christopher Zimmermann [ 17/Jan/17 ]

This would be useful.
I am concerned that the UI indication should be compatible with possible future improvements to display or somehow indicate the "virtual pages" that are typically created by merging Content app content with a single page, based on a URL parameter.
For example the tour pages in the travel demo, there is one page in the Pages app - but on the website there are many pages based on it (one for each tour.)
(https://demopublic.magnolia-cms.com/tours/magnolia-travels/A-Taste-of-Malaysia.html)

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.
I agree that the icons have to be aligned vertically so problems stand out visually.

Solution

  • Option 1: either create a new column named „problems“ and place the errors inside that column or
  • Option 2: align the icon to the right side inside the column „page“.

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

  • Tooltip is green color according to the styleguide.
  • Text in tooltip should be even less technical (see mockup).
  • Ideally with a click, the user would be taken to a page where the problem can be solved. Not sure if this can be implemented.
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.
That would also require to update the formatter provided by personalization. I'm not saying it's not possible but need an extra work.

Comment by Jan Haderka [ 18/Jan/17 ]

avongunten thanks, I'll see what can be done in that regard
rkovarik well, perhaps it's time to find a way to split that code and composite it at runtime? (not saying that it would be easy or fast )

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 ]

Already done (ignore the snowing).

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.
Tooltips set from the Vaadin side (via fields' description or tables' ItemDescriptionGenerator) are not native tooltips, just custom overlays produced by Vaadin; those are 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).
The formatter usually adds appropriate class for use in the selector. Btw, here it says activation-status. If the inherited rules are really needed, then we'd abstract those as a "column-status" class or so.

Generated at Mon Feb 12 06:15:48 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.