Code review comment for ~rmescandon/snappy-hwe-snaps/+git/wifi-connect:config-ui

Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

> > Adding some notes from external conversation:
> >
> > config content should be always available (not just on first use)
> This PR focus on first config
>
>
> >
> > config content should show on first use of management portal and be
> available
> > from management portal at any later time
> That has to be in another PR. This one only covers first config. Otherwise
> this PR can last forever adding new features not required at beginning.
>

Are you saying this PR only displays config UI on first use? That's not the overall spec'd behavior. In any case, if you are planning to leave out normal configuration after first use, please create a trello card for this.

>
> >
> > config content should physically be part of management.html (not in a
> separate
> > html page). Visibility of parts is controlled by css appropriately.
> I don't see the reason for this. Actually I'm completely against having all
> the portal in only one html template. However, so it is made.

Can you please provide your reasoning?

>
>
>
> >
> > Just to be clear, pre-config.json should NEVER be written to since it
> > represents the system-integrator's pre config via gadget, if any
> It is not written in any case
>
> >
> > The only value you need to read from pre-config.json is portal.no-reset-
> creds.
> > If this is true, then the user is not required to reset the portal password
> > and the wifi passphrase on first use.
> It is also reading portal.no-operational, though it's not been used so far
> anywhere

Can you drop the no-operational part? (You may have added that when you thought we wanted a Checkbox to not display the oper portal, but really that's never going to be configured by this UI.

>
> >
> > I don't think we need to write any config.json. Any changed values should be
> > set after form validates in wifi-ap (and the portal password if that is set)
> There is no config.json
>
>
> >
> > All params on the config page except for portal.password should get their
> > values for display from wifi-ap (no local storage of these values)
> All remote values are taken from wifi-ap, local ones from pre-config.json,
> though in config page any local one is populated

Which local ones are you populating?

thx
>
>
> >
> > Emtpy fields (except for portal password and wifi passprhase) should cause
> > form validation error.
> They do
>
> >
> > After form validates, only changed params should be set. This prevents wifi-
> ap
> > DOWN>UP if not needed.
> So it works

« Back to merge proposal