RFR: JMC-5473: Quick search for Automated Analysis Result Table

[Carmine Vincenzo Russo]

Hi Jie,

Thanks for pointing out these errors, I'll fix them and get back with an
updated version.

Thanks,
Carmine

On Thu, Aug 1, 2019 at 4:16 PM Jie Kang <jkang at redhat.com> wrote:

> On Thu, Aug 1, 2019 at 6:53 AM Carmine Vincenzo Russo
> <carusso at redhat.com> wrote:
> >
> > Hi,
> >
> > The attached patch addresses the issue JMC-5473[0] in which a search
> > feature for the Automated Analysis Result Table was lacking.
> >
> > In ResultOverview.java, I added the text component and a simple layout to
> > give the page more consistency when the table is displayed.
> > I also produced ResultOverviewTest.java to test if the table has
> > components, the search feature operates, and two more to check if the
> > behaviour is what we expect with a nonsense search and a specific search.
> >
> > A side note, since there were no tests for the Automated Analysis
> Result, I
> > had to add the tab in JfrUi and allow the record analysis in two other
> > classes, OldRecordingsVerificationTest.java and JfrRecordingTest.java,
> > because they go through all the tabs listed in JfrUi during their tests.
> >
> > How does it look?
>
> Hi Carmine,
>
> Thanks for doing this; the tests are appreciated! I have some issues
> to address below:
>
> diff --git
> a/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/overview/R
> esultOverview.java
>
> b/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/ov
> erview/ResultOverview.java
> ---
> a/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/overview/ResultOv
> erview.java
> +++
> b/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/overview/ResultOv
> erview.java
> [...]
> -
> +
>
> * Whitespace additions like the example shown above should be removed;
> there are others in the patch.
>
> -                       Map<Result, DataPageDescriptor> map =
> createResultMap();
> +                       GridLayout layout = new GridLayout(1, true);
> +                       this.form.getBody().setLayout(layout);
> +                       text = new Text(form.getBody(), SWT.BORDER |
> SWT.HORIZONTAL | SWT.SEARCH | SWT.RESIZE);
> +                       text.setLayoutData(new GridData(SWT.FILL,
> SWT.DEFAULT, true, false));
> +
>  text.setMessage(Messages.ResultOverview_SEARCH_TABLE);
> +
>  text.setToolTipText(Messages.ResultOverview_SEARCH_BAR);
> +                       Map<Result, DataPageDescriptor> map =
> createResultMap(text.getText());
>                         table = new ResultTableUi(form, toolkit,
> editor, loadedState, map);
> +                       ModifyListener listener = new ModifyListener() {
> +                               @Override
> +                               public void modifyText(ModifyEvent e) {
> +                                       Map<Result,
> DataPageDescriptor> map = createResultMap(text.getText());
> +                                       table.updateInput(map);
> +                               }
> +                       };
> +                       text.addModifyListener(listener);
>                 }
> * The additions to the Table UI done above should occur in the Table
> UI code, rather than in the overview which is managing the HTML UI and
> the Table UI, ie. the above should be within ResultTableUi.java
>
> * When switching to the Table and then back to the HTML, the resulting
> view is strange; see the linked image [1]. This may have to do with
> how the form's layout is modified by your addition. This form is the
> parent for the ResultTableUi. I would add children to it with custom
> layouts, but I would not modify the 'parent' layout itself.
>
> https://imgur.com/a/nS1m2T2
>
>
> Regards,
>
> >
> > Regards,
> > Carmine
> >
> > [0] https://bugs.openjdk.java.net/browse/JMC-5473
> >
> > --
> > Carmine Vincenzo Russo
>


-- 
Carmine Vincenzo Russo