V4: review request for configure details page

* V4: review request for configure details page
@ 2014-02-28  1:08 Reyna, David
  2014-02-28  6:40 ` Reyna, David
  2014-02-28 10:42 ` Barros Pena, Belen
  0 siblings, 2 replies; 10+ messages in thread
From: Reyna, David @ 2014-02-28  1:08 UTC (permalink / raw)
  To: Barros Pena, Belen, Damian, Alexandru; +Cc: Paul Eggleton, toaster

Hi Belen,

On to V4 for "dreyna/configure-detail-view"!

FIXES:

* The description filter tooltip says: "Showing only Show only variables..."
* I see: FILTER:conf/<filter_applied>
* variable values over 150 characters are not truncated on the table.
* Extended variable name patterns should link to the root variable entry 

All done!

* When using the search, each row will appear multiple times.

Done! I realized that the "searches" were in fact working inside the vhistory list (it is column sorts that are broken). The unexpected consequence is that each hit added the record to the queryset, with the observed anomaly that multiple file name hits added duplicate rows in the table. Here is my resolution.

    # remove duplicate records from multiple search hits in the VariableHistory table
    queryset = queryset.distinct()

* Variables with no value and no file should be excluded

Done! My tests show this working as expected, where I do see rows with a file but no value, but I do not see rows without both. Plus, since this is done in the view and not in the template, the pagination works as expected. Here is my resolution.

    # remove records where the value is empty AND there are no history files
    queryset = queryset.exclude(variable_value='',vhistory__file_name__isnull=True)    

I had to play around with this to get the second clause to work, and I would like Alex to review it, but as I said it seems to work.

PENDING ISSUES:

> Where should I map the help for variable name with patterns of the form "do_*"

I sent email on this question separately.

> "My only doubts are about case 2, where
> I am not sure it makes sense to show the last file when the search string
> is found in any other file for a certain variable."

I propose that most user searches are for variable names, so the file column will appear to reflect the default behavior. If I do not by default include the last file, then searching for variable names will _always_ result in the file column becoming empty (expect by coincidence), which does not look like the normal behavior for the page. 

It may only look odd if you do a search for example for "conf/bitbake", where you would get all of the matches _plus_ the last file. Given the downside in the previous and more common use case, I think that this little bit of extra information is worth it.

I think a match between both a variable and a file name is unlikely.

>  * The column sort for files will only test the first file in any vhistory list.
>  * When you cancel the search string, the column filters are also reset.

Still pending.

> Any scripting inside a template should go at the very end of the
> markup, but before the closing </body> tag.

All I am saying is that unless we define an additional block for template-specific scripts (and I think that at some point this would be a good idea), both the "<head>...</head>" and the "<body>...</body>" are locked inside "main.html" and are inaccessible to template files, and thus inaccessible for template-specific scripts.

Thanks!
David

> -----Original Message-----
> From: Barros Pena, Belen [mailto:belen.barros.pena@intel.com]
> Sent: Thursday, February 27, 2014 4:05 AM
> To: Reyna, David; Damian, Alexandru
> Cc: toaster@yoctoproject.org; Paul Eggleton
> Subject: Re: V3: review request for configure details page
> 
> Hi David,
> 
> Thanks for all the good work. I have commented inline on your email when
> required.
> 
> There are also a couple of small issues I am still seeing:
> 
> * The description filter tooltip says: "Showing only Show only variables
> with description". It should say "Showing only variables with
> description". If needed, we can change the radio button label to say just
> "Variables with description²
> 
> * Variables with no value and no file are only there because there is a
> description set for them in documentation.conf. They should be excluded
> from the view (table, search, sorting and filtering).
> 
> 
> Cheers
> 
> Belén
> 
> On 27/02/2014 03:53, "Reyna, David" <david.reyna@windriver.com> wrote:
> 
> >Hi Belen,
> >
> >I have implemented the proposed design changes to the "Configure" page.
> >
> >   dreyna/configure-detail-view
> >
> >I did need to make some small changes to the proposed design, as
> >documented below. There are also some outstanding issues, which I also
> >list below.
> >
> >IMPLEMENTATION:
> >
> >> 1) "Showing history for all variables"
> >
> >Done.
> >
> >> 2) "Searching and filtering" (Hidden file problem)
> >
> >Done.
> >
> >I did however realize that "search" applies to all fields, not just the
> >file list. This is an issue when you for example search the rpwsa for
> >"BBINCLUDED", because suddenly all of the files would disappear because
> >none would match. I do not think that this is what the user would expect
> >when they are searching for variables and not file names.
> >
> >Here is therefore how I implemented the file filtering:
> >
> >  'search' 'file filter' Action
> >  -------- ------------- ----------------------
> >    -        -           show last file
> >  value      -           show last file, plus any file names that match
> >search
> >    -      value         show only file names that match filter
> >  value    value         show any file names that match either string
> >
> >I think that this meets out needs, and is a "least surprise" for users.
> 
> In general, this makes sense to me. My only doubts are about case 2, where
> I am not sure it makes sense to show the last file when the search string
> is found in any other file for a certain variable.
> 
> Also, when I apply a 'set in file' filter, I see the following string on
> each cell: FILTER:conf/<filter_applied>
> 
> >
> >> 3) "Dealing with long values"
> >
> >Done. To see it work, turn off the "Description" filter and look at the
> >variable "BBINCLUDED", which is guaranteed to be bigger than 570
> >characters.
> 
> This is working in the history modal dialogs, but variable values over 150
> characters are not truncated on the table. It's important we do this in
> order to keep the table readable.
> 
> >
> >> 4) "What if a variable value is an empty string?"
> >
> >Done.
> >
> >> 5) "What do we do with the 613 B_* variables?"
> >
> >They are now gone from the view.
> >
> >> 6) "Linking variables to the Yocto Project reference manual"
> >
> >Done.
> >
> >However, the statement "we do know that all variables with a description
> >in documentation.conf have an entry in
> >the Yocto Project reference manual" is not universally true. Here are
> >some counter examples:
> >
> >  ALLOW_EMPTY_${PN}-dbg
> >  ASSUME_PROVIDED
> >  ASSUME_SHLIBS
> >  BBFILE_PATTERN_core
> >  BBFILE_PRIORITY_core
> >  ...
> >
> >Some of these are obviously extended patterns, but the code does not know
> >that, and there is no obvious way to filter for that.
> 
> Extended patterns should link to the root variable entry (so
> ALLOW_EMPTY_${PN}-dbg links to the ALLOW_EMPTY entry). But you are right:
> ASSUME_PROVIDED does not have an entry in the manual. I'll review the
> content of documentation.conf to eliminate any mismatches.
> 
> >
> >
> >LIMITATIONS:
> >
> >7) There are still the observed defects. I will investigate, but they are
> >outside of the code under review.
> >
> >  * When you cancel the search string, the column filters are also reset.
> >  * The search will only test the first file in any vhistory list.
> >  * When using the search, each row will appear multiple times.
> >
> >8) I have added the new Javascript code to support the value extender
> >dialog to the file "templates/base.html", and it works!
> 
> It does work wonderfully :)
> 
> >
> >
> >I do not know if this the best place. Nor do I know if it will affect
> >other pages that would not otherwise use it.
> >
> >I do observe that there is currently no way for a page to add local
> >script code to the "<head>" section, which is currently locked inside
> >"base.html". Maybe you can add script code to other sections, but I know
> >that in testing that this new script section would not work when placed
> >anywhere within "templates/configvars.html".
> 
> Any scripting that will be reused by several templates should go on
> main.js. Any scripting inside a template should go at the very end of the
> markup, but before the closing </body> tag. That way the page loads first
> all markup and we show something to users, while the scripts take their
> time to load. But I think in general we should aim to have all js neatly
> put away in main.js
> 
> >
> >Thanks,
> >David
> >
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread