JMC-5372: Exception printed on page when opening invalid recording

[Alex Macdonald]

Ping - I sent this out about a week ago, let me know what you think.

On Wed, Nov 14, 2018 at 4:12 PM Alex Macdonald <almacdon at redhat.com> wrote:

> Hi Marcus,
>
> On Tue, Nov 13, 2018 at 7:59 PM, Marcus Hirt <marcus.hirt at oracle.com>
> wrote:
>
>> Hi Alex,
>>
>> Personally I am a bit allergic to modal dialogs. For example, if we have
>> multiple recordings ending with problems, I'd rather have the editors
>> opening
>> with details than one or more modal dialogs. That is, of course, a
>> personal
>> preference that I'd be happy to discuss here. That said, I think what is
>> shown,
>> and how it is shown, in the editor could be improved. For example we
>> should
>> show the error title and message (that would have been showed in the
>> modal
>> dialog) first, followed by the stack trace. Possibly not throwing the
>> stack trace in the user's face until it is asked for.
>>
>
> That sounds fair enough to me. I've taken what you said into account and
> made a couple of modifications to the error page.
>
> I've transferred the text explaining the error from the dialog to the
> error page, and removed the dialog modal. The dialog title text was very
> similar to the current error message printed on the screen, so I've only
> kept the latter to avoid redundancy. In response to your comment about
> showing the stack trace when it's asked for, the stack trace is now hidden
> under a expandable component so it can be collapsed & expanded as desired.
>
> I've included some images to show these changes.
>
> Error page (gif): https://imgur.com/g8hpXlg
> Error page (collapsed): https://imgur.com/irTarA6
> Error page (expanded): https://imgur.com/M51z8ct
>
> Cheers,
>
> Alex
>
>
>> Please let me know what you think!
>>
>> Kind regards,
>> Marcus
>>
>> On 2018-11-13, 21:45, "jmc-dev on behalf of Alex Macdonald" <
>> jmc-dev-bounces at openjdk.java.net on behalf of almacdon at redhat.com> wrote:
>>
>>     Hi,
>>
>>     This short patch addresses JMC-5372 [0], in which the error dialog is
>> not
>>     displayed when an exception is thrown in the JFR Editor.
>>
>>     As far as I can tell, the error here is within the catch block of the
>>     "displayPage" function in the JFR Editor [1]. When an exception is
>> caught
>>     there is an evaluation of the boolean property for
>> "showModalErrorDialog"
>>     [2], however I cannot find the instance where this property would be
>>     toggled from false to true. As a result, the if-statement takes the
>> branch
>>     that displays the error page, but not the error dialog. The proposed
>> fix
>>     here removes the check for the boolean (because we have already
>> caught the
>>     exception), and instead opt to display both the error page and the
>> modal.
>>
>>     I've included a couple of images to show the result of this patch
>> [3], as
>>     well as gifs showing the before [4] & after [5] experience.
>>     Before (gif): https://imgur.com/hHRmkx3 [3]
>>     After (gif): https://imgur.com/8rtaysS [4]
>>     After (img): https://imgur.com/0oBi6nH [5]
>>
>>     Thoughts?
>>
>>     Cheers,
>>
>>     Alex
>>
>>     [0] https://bugs.openjdk.java.net/browse/JMC-5372
>>     [1]
>>
>> http://hg.openjdk.java.net/jmc/jmc/file/a76a464b3764/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/JfrEditor.java#l240
>>     [2]
>>
>> http://hg.openjdk.java.net/jmc/jmc/file/a76a464b3764/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/JfrEditor.java#l243
>>     [3] https://imgur.com/hHRmkx3
>>     [4] https://imgur.com/8rtaysS
>>     [5] https://imgur.com/0oBi6nH
>>
>>
>>
>>
>>
>