V2: [review-request] configure variables page: dreyna/hardcoded_submit_7246_7371

* V2: [review-request] configure variables page: dreyna/hardcoded_submit_7246_7371
@ 2015-02-28  4:45 Reyna, David
  2015-03-02 11:36 ` Barros Pena, Belen
  0 siblings, 1 reply; 5+ messages in thread
From: Reyna, David @ 2015-02-28  4:45 UTC (permalink / raw)
  To: BARROS PENA, BELEN; +Cc: toaster

Hi Belén.

Ok, I have fixed the issues and re-pushed my commit "dreyna/hardcoded_submit_7246_7371".

[x] "If you add manually one of our default variables, I can briefly see an
error alert coming up, then disappearing very quickly."

This was a race condition between the 'postEditAjaxRequest' and the page reload. Fixed.

[x] "When you add back SDKMACHINE, when you click the 'change' icon the
Cancel button is disabled"
[x] "When you add back PACKAGE_CLASESS, the 'Save' button is disabled, so you
cannot make any changes to the variable value"

This was really odd and took time to figure out. It turns out that with a soft reload these two buttons (and only these two buttons) get a disabled="" attribute inserted magically. The fix was to do a hard reload instead (which is anyway better in the long run).

[x] "... I found the message a bit confusing ..."

I went with the generic: "You cannot set this variable here. Please set it in the project main page", with a link to that project's main page.

[x] "The only issue I've found is that the MACHINE variable is showing up ..."

Filter added for both the blacklist and {MACHINE,BBLAYERS} in the initial page serve and the XHR serve.

- David

> -----Original Message-----
> From: Barros Pena, Belen [mailto:belen.barros.pena@intel.com]
> Sent: Friday, February 27, 2015 5:15 AM
> To: Reyna, David
> Cc: toaster@yoctoproject.org; DAMIAN, ALEXANDRU
> Subject: Re: [Toaster] [review-request] configure variables page:
> dreyna/hardcoded_submit_7246_7371
> 
> I just found a couple of other things, sorry:
> 
> * If you add manually one of our default variables, I can briefly see
> an
> error alert coming up, then disappearing very quickly.
> 
> * When you add back SDKMACHINE, when you click the 'change' icon the
> Cancel button is disabled.
> 
> * When you add back PACKAGE_CLASESS, the 'Save' button is disabled, so
> you
> cannot make any changes to the variable value
> 
> Cheers
> 
> Belén
> 
> 
> 
> On 27/02/2015 12:10, "Barros Pena, Belen" <belen.barros.pena@intel.com>
> wrote:
> 
> >Hi David,
> >
> >This is working great. I only have a couple of small comments
> (inline):
> >
> >On 27/02/2015 04:08, "Reyna, David" <david.reyna@windriver.com> wrote:
> >
> >>Hi Belén,
> >>
> >>This commit ³dreyna/hardcoded_submit_7246_7371³ fixes these items:
> >>
> >>  7246 ³Hardcoded variables in /configuration page²
> >
> >The only issue I've found is that the MACHINE variable is showing up
> in
> >the page. I think we should filter it out, particularly because the
> >validation you have added to the 'add variable' form for preventing
> people
> >setting the machine in this page.
> >
> >>  7371 ³Toaster managed mode: configuration variables page allows
> empty
> >>inputs into database²
> >>
> >>
> >>  * Finally, I added/fixed code to enforced the blocking of the
> variables
> >>that are managed outside this page, specifically BBLAYERS and MACHINE
> >>which are handled by the project detail page.
> >
> >This is very nice, I found the message a bit confusing, because it
> gives
> >me a reason but does not tell me what to do. I know this might require
> >some hard coding, but could we have different messages for the MACHINE
> and
> >the BBLAYERS cases?
> >
> >So if you try to set the MACHINE, the message should say:
> >
> >"You cannot set this variable here. Please set the MACHINE in the
> project
> >main page"
> >
> >If you try to set BBLAYERS, the message should say:
> >
> >"You cannot set this variable here. Please add and remove layers in
> the
> >project main page"
> >
> >If this is too much and you rather use a single message, it could say:
> >
> >"You cannot set this variable here. Please set it in the project main
> >page"
> >
> >In all the above messages, "project main page" should be a link to the
> >project page.
> >
> >
> >I hope I am not asking too much.
> >
> >Thanks!!
> >
> >Belén
> >
> >>
> >>- David
> >>
> >>
> >
> >--
> >_______________________________________________
> >toaster mailing list
> >toaster@yoctoproject.org
> >https://lists.yoctoproject.org/listinfo/toaster
> 

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