💄 Transition validation markup #666
Conversation
| @@ -1355,7 +1362,7 @@ function DisplayNavigatorGroupTab($oP) | |||
| $sMessage = $e->getMessage(); | |||
| $sSeverity = 'info'; | |||
| } | |||
| $sIssueDesc = Dict::Format('UI:ObjectCouldNotBeWritten',$sIssues); | |||
| $sIssueDesc = ($oException !== null ? '<div style="display: block;"><style scoped> ul { list-style-type: disc; } </style>'.$oException->getHtmlMessage().'</div>' : Dict::Format('UI:ObjectCouldNotBeWritten', $sIssues)); | |||
There was a problem hiding this comment.
While not ideal, this will be easier to maintain than a CSS rule somewhere in a file.
The main issues:
- Error modal uses flex.
- The "ul" element loses its standard formatting by default in iTop.
There was a problem hiding this comment.
ErrorModal should be fixed IMHO
There was a problem hiding this comment.
FYI, in the backoffice when you want HTML to retrieve its standard style like for lists, surround it with the .ibo-is-html-content.
I would strongly be against inline style like this. A dedicated rule in the right file would be better.
That being said, you should ping @jf-cbd or @rquetiez , they worked on error messages and removing/sanitizing HTML errors last summer.
There was a problem hiding this comment.
I'll try the .ibo-is-html-content when I find some time.
Not sure if that would also change the display to block or if there's another CSS class which we can use.
| $sIssues = $e->getMessage(); | ||
|
|
||
| if($e instanceof CoreCannotSaveObjectException) { | ||
| $oException = $e; | ||
| } |
There was a problem hiding this comment.
What if you would do this instead? Then there's no need to do the if/else later on.
| $sIssues = $e->getMessage(); | |
| if($e instanceof CoreCannotSaveObjectException) { | |
| $oException = $e; | |
| } | |
| $sIssues = $e->getHtmlMessage(); |
(correct styling would still something to be done in CSS, maybe separate PR)
There was a problem hiding this comment.
I'm not sure whether or not we can expect other exceptions here, which don't support the getHtmlMessage(), so I was being cautious :)
There was a problem hiding this comment.
Right, CoreException doesn’t seem to have this method 🤔
Then maybe just do getHtmlMessage when it exists and the normal getMessage otherwise?
|
Accepted during functional review. Will check with @rquetiez for technical details (3.2 compatibility and way to define the message). |
Base information
Symptom (bug) / Objective (enhancement)
When using for example the new event listeners to perform a check before the object is written during a transition; the current error modal is ugly. It does not use a nice list; and it shows context data.
Proposed solution (bug and enhancement)
The error messages are just examples.
Before:
After:
Checklist before requesting a review
Checklist of things to do before PR is ready to merge