kurono:
> sajolida:
>> kurono:
>>> Hi Sajolida,
>>>
>>> Ok, I have applied all the suggested changes, and I think its time to a
>>> code review as well:
>>>
>>> https://git-tails.immerda.ch/kurono/liveusb-creator/log/?h=feature/9005-Improve-tails-installer
>>
>> So I finally found the time to review this. Sorry for the huge delay!
>> And thanks for the testing tips. I also realized that they have to be
>> run with sudo (at least from Tails), so what about adding these
>> instructions to the repo?
>
> Thanks!
> which repo?
I did that in #12344. If you don't mind reviewing it.
>> git clone https://git-tails.immerda.ch/liveusb-creator
>> sudo apt-get install python-distutils-extra python3-distutils-extra
>> sudo python setup.py install
>> sudo mkdir /usr/local/share/tails-installer
>> sudo cp -R data/* /usr/local/share/tails-installer/
>> sudo tails-installer
>
> this last one is not necessary, with ./tails-installer is enough.
Indeed. Before I tried with 'tails-installer' which fails.
>> I'm sending a few screenshots in attachment for reference.
>>
>> Regarding my tests and review:
>>
>> 1. I propose defaulting to "Clone" instead of "Use ISO" when running
>> from Tails because I expect it to be the most frequently used option
>> (and that's why it's on top).
>
> Done.
Ack.
>> 2. What about deactivating (graying out) the file selection unless "Use
>> ISO" is selected? See:
>>
>> https://mailman.boum.org/pipermail/tails-ux/attachments/20160629/62e60034/attachment-0003.png
>
> Done.
Ack.
>> 3. If "Use ISO" is selected but no ISO image is specified, I could still
>> click "Install" (but then I don't know what it's doing). I think we
>> should then display the dialog for selecting the ISO image before asking
>> for confirmation before install. This would be the "forgiving" option.
>> Another option would be to add a constrain and gray out the "Install"
>> button until an ISO image is selected. But I think I prefer the first
>> option.
>
> I have implemented another option:
> Showing an alert message when the install button or the reinstall link
> are pressed, without selecting the ISO image file, if that option is chosen.
I'm fine with that. I rewrote a bit the message with 11ebad8.
Also, since we're there, what about allowing the user to select the ISO
image from this dialog to be more forgiving?
It would allow her to solve the problem directly from the error message
and avoid her to have to go back to the main window and figure out what
this is referring to.
What about, instead of having only an "Ok" button, having:
[Cancel] [Select ISO image...]
But that's optional and you can create a ticket to do that later on if
you prefer.
>> 4. Did you implement changing the "Install" button into "Upgrade" (and
>> "Reinstall")? Because I didn't see this when doing my tests...
>
> Yes sure, it works for me, it only is shown when the USB stick already
> has tails installed. Send me please a screenshot when this happens to you.
Some more remarks:
1. If I start Tails Installer without a target USB stick and then plug
it in, the interface gets widely resized. It not a serious problem but
looks unpolished. See before.png and after.png.
Also, the description of the USB stick in the main window and in the
confirmation dialog is different. See confirm.png. I prefer the one in
the confirmation dialog because it is shorter while still having all the
necessary info (vendor, size, and model) but without the long number
that is displayed in the main window. Maybe getting rid of the long
number in the main window would also help solving the resizing issue.
What do you think?
2. By default the file selector is too small to display the full name of
the ISO image and only displays "tails-...".
If I am playing with different versions (for example when upgrading) I
would like to have some feedback from the main window about the ISO
image that I selected.
Would it work to make the file selector wide enough to handle our usual
filename length: ~22 characters for "tails-archh-x.yy.z.iso"?
3. In the confirmation dialog before installation or upgrade, I would
love to have, instead of the [No] [Yes] buttons:
[Cancel] [Install]
or
[Cancel] [Upgrade]
I'm myself always a bit scared when upgrading a Tails USB stick and this
would be reassuring (even if we're already solving this problem is other
ways with the rest of your work).
4. Would it be easy to fix the minor alignment issues on the right,
especially with the links? See alignment.png. Otherwise forget it :)
5. Would it be possible to display "The persistent storage on this USB
stick will be preserved." only if there actually is a persistent
storage? But if that requires tweaking the backend we can create a
ticket to do that later on.
6. If I plug two USB sticks, one with Tails and one without, and switch
from Upgrade to Install and then back to Upgrade, then the Reinstall
link disappears.
7. I rephrase the confirmation dialog quite a bit with e1908d0. Tell me
what you thing. Especially since I'm tweaking Gtk a bit to display an
empty new line. I don't know if that's good or if it's the proper way to
do that.
But hey, we're almost there and I'm really excited to see this finally
tackled in Tails Installer which is pretty much the first piece of
software our users have to interact with. Good job!
,%20so%20what%20about%20adding%20these%0A>%20>>%20instructions%20to%20the%20repo?%0A>%20>%20%0A>%20>%20Thanks!%0A>%20>%20which%20repo?%0A>%20%0A>%20I%20did%20that%20in%20#12344.%20If%20you%20don't%20mind%20reviewing%20it.%0A>%20%0A>%20>>%20git%20clone%20https://git-tails.immerda.ch/liveusb-creator%0A>%20>>%20sudo%20apt-get%20install%20python-distutils-extra%20python3-distutils-extra%0A>%20>>%20sudo%20python%20setup.py%20install%0A>%20>>%20sudo%20mkdir%20/usr/local/share/tails-installer%0A>%20>>%20sudo%20cp%20-R%20data/*%20/usr/local/share/tails-installer/%0A>%20>>%20sudo%20tails-installer%0A>%20>%20%0A>%20>%20this%20last%20one%20is%20not%20necessary,%20with%20./tails-installer%20is%20enough.%0A>%20%0A>%20Indeed.%20Before%20I%20tried%20with%20'tails-installer'%20which%20fails.%0A>%20%0A>%20>>%20I'm%20sending%20a%20few%20screenshots%20in%20attachment%20for%20reference.%0A>%20>>%0A>%20>>%20Regarding%20my%20tests%20and%20review:%0A>%20>>%0A>%20>>%201.%20I%20propose%20defaulting%20to%20%22Clone%22%20instead%20of%20%22Use%20ISO%22%20when%20running%0A>%20>>%20from%20Tails%20because%20I%20expect%20it%20to%20be%20the%20most%20frequently%20used%20option%0A>%20>>%20(and%20that's%20why%20it's%20on%20top).%0A>%20>%20%0A>%20>%20Done.%0A>%20%0A>%20Ack.%0A>%20%0A>%20>>%202.%20What%20about%20deactivating%20(graying%20out)%20the%20file%20selection%20unless%20%22Use%0A>%20>>%20ISO%22%20is%20selected?%20See:%0A>%20>>%0A>%20>>%20https://mailman.boum.org/pipermail/tails-ux/attachments/20160629/62e60034/attachment-0003.png%0A>%20>%20%0A>%20>%20Done.%0A>%20%0A>%20Ack.%0A>%20%0A>%20>>%203.%20If%20%22Use%20ISO%22%20is%20selected%20but%20no%20ISO%20image%20is%20specified,%20I%20could%20still%0A>%20>>%20click%20%22Install%22%20(but%20then%20I%20don't%20know%20what%20it's%20doing).%20I%20think%20we%0A>%20>>%20should%20then%20display%20the%20dialog%20for%20selecting%20the%20ISO%20image%20before%20asking%0A>%20>>%20for%20confirmation%20before%20install.%20This%20would%20be%20the%20%22forgiving%22%20option.%0A>%20>>%20Another%20option%20would%20be%20to%20add%20a%20constrain%20and%20gray%20out%20the%20%22Install%22%0A>%20>>%20button%20until%20an%20ISO%20image%20is%20selected.%20But%20I%20think%20I%20prefer%20the%20first%0A>%20>>%20option.%0A>%20>%20%0A>%20>%20I%20have%20implemented%20another%20option:%0A>%20>%20Showing%20an%20alert%20message%20when%20the%20install%20button%20or%20the%20reinstall%20link%0A>%20>%20are%20pressed,%20without%20selecting%20the%20ISO%20image%20file,%20if%20that%20option%20is%20chosen.%0A>%20%0A>%20I'm%20fine%20with%20that.%20I%20rewrote%20a%20bit%20the%20message%20with%2011ebad8.%0A>%20%0A>%20Also,%20since%20we're%20there,%20what%20about%20allowing%20the%20user%20to%20select%20the%20ISO%0A>%20image%20from%20this%20dialog%20to%20be%20more%20forgiving?%0A>%20%0A>%20It%20would%20allow%20her%20to%20solve%20the%20problem%20directly%20from%20the%20error%20message%0A>%20and%20avoid%20her%20to%20have%20to%20go%20back%20to%20the%20main%20window%20and%20figure%20out%20what%0A>%20this%20is%20referring%20to.%0A>%20%0A>%20What%20about,%20instead%20of%20having%20only%20an%20%22Ok%22%20button,%20having:%0A>%20%0A>%20%0A>%20But%20that's%20optional%20and%20you%20can%20create%20a%20ticket%20to%20do%20that%20later%20on%20if%0A>%20you%20prefer.%0A>%20%0A>%20>>%204.%20Did%20you%20implement%20changing%20the%20%22Install%22%20button%20into%20%22Upgrade%22%20(and%0A>%20>>%20%22Reinstall%22)?%20Because%20I%20didn't%20see%20this%20when%20doing%20my%20tests...%0A>%20>%20%0A>%20>%20Yes%20sure,%20it%20works%20for%20me,%20it%20only%20is%20shown%20when%20the%20USB%20stick%20already%0A>%20>%20has%20tails%20installed.%20Send%20me%20please%20a%20screenshot%20when%20this%20happens%20to%20you.%0A>%20%0A>%20Some%20more%20remarks:%0A>%20%0A>%201.%20If%20I%20start%20Tails%20Installer%20without%20a%20target%20USB%20stick%20and%20then%20plug%0A>%20it%20in,%20the%20interface%20gets%20widely%20resized.%20It%20not%20a%20serious%20problem%20but%0A>%20looks%20unpolished.%20See%20before.png%20and%20after.png.%0A>%20%0A>%20Also,%20the%20description%20of%20the%20USB%20stick%20in%20the%20main%20window%20and%20in%20the%0A>%20confirmation%20dialog%20is%20different.%20See%20confirm.png.%20I%20prefer%20the%20one%20in%0A>%20the%20confirmation%20dialog%20because%20it%20is%20shorter%20while%20still%20having%20all%20the%0A>%20necessary%20info%20(vendor,%20size,%20and%20model)%20but%20without%20the%20long%20number%0A>%20that%20is%20displayed%20in%20the%20main%20window.%20Maybe%20getting%20rid%20of%20the%20long%0A>%20number%20in%20the%20main%20window%20would%20also%20help%20solving%20the%20resizing%20issue.%0A>%20What%20do%20you%20think?%0A>%20%0A>%202.%20By%20default%20the%20file%20selector%20is%20too%20small%20to%20display%20the%20full%20name%20of%0A>%20the%20ISO%20image%20and%20only%20displays%20%22tails-...%22.%0A>%20%0A>%20If%20I%20am%20playing%20with%20different%20versions%20(for%20example%20when%20upgrading)%20I%0A>%20would%20like%20to%20have%20some%20feedback%20from%20the%20main%20window%20about%20the%20ISO%0A>%20image%20that%20I%20selected.%0A>%20%0A>%20Would%20it%20work%20to%20make%20the%20file%20selector%20wide%20enough%20to%20handle%20our%20usual%0A>%20filename%20length:%20~22%20characters%20for%20%22tails-archh-x.yy.z.iso%22?%0A>%20%0A>%203.%20In%20the%20confirmation%20dialog%20before%20installation%20or%20upgrade,%20I%20would%0A>%20love%20to%20have,%20instead%20of%20the%20%5BNo%5D%20%5BYes%5D%20buttons:%0A>%20%0A>%20%0A>%20or%0A>%20%0A>%20%0A>%20I'm%20myself%20always%20a%20bit%20scared%20when%20upgrading%20a%20Tails%20USB%20stick%20and%20this%0A>%20would%20be%20reassuring%20(even%20if%20we're%20already%20solving%20this%20problem%20is%20other%0A>%20ways%20with%20the%20rest%20of%20your%20work).%0A>%20%0A>%204.%20Would%20it%20be%20easy%20to%20fix%20the%20minor%20alignment%20issues%20on%20the%20right,%0A>%20especially%20with%20the%20links?%20See%20alignment.png.%20Otherwise%20forget%20it%20:)%0A>%20%0A>%205.%20Would%20it%20be%20possible%20to%20display%20%22The%20persistent%20storage%20on%20this%20USB%0A>%20stick%20will%20be%20preserved.%22%20only%20if%20there%20actually%20is%20a%20persistent%0A>%20storage?%20But%20if%20that%20requires%20tweaking%20the%20backend%20we%20can%20create%20a%0A>%20ticket%20to%20do%20that%20later%20on.%0A>%20%0A>%206.%20If%20I%20plug%20two%20USB%20sticks,%20one%20with%20Tails%20and%20one%20without,%20and%20switch%0A>%20from%20Upgrade%20to%20Install%20and%20then%20back%20to%20Upgrade,%20then%20the%20Reinstall%0A>%20link%20disappears.%0A>%20%0A>%207.%20I%20rephrase%20the%20confirmation%20dialog%20quite%20a%20bit%20with%20e1908d0.%20Tell%20me%0A>%20what%20you%20thing.%20Especially%20since%20I'm%20tweaking%20Gtk%20a%20bit%20to%20display%20an%0A>%20empty%20new%20line.%20I%20don't%20know%20if%20that's%20good%20or%20if%20it's%20the%20proper%20way%20to%0A>%20do%20that.%0A>%20%0A>%20But%20hey,%20we're%20almost%20there%20and%20I'm%20really%20excited%20to%20see%20this%20finally%0A>%20tackled%20in%20Tails%20Installer%20which%20is%20pretty%20much%20the%20first%20piece%20of%0A>%20software%20our%20users%20have%20to%20interact%20with.%20Good%20job!%0A>%20 "Reply to this message")
