Feature #15369

Improve buttons of tails-screen-locker

Added by sajolida 2018-03-02 19:14:43 . Updated 2018-03-14 11:04:14 .

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Target version:
Start date:
2018-03-02
Due date:
% Done:

100%

Feature Branch:
bugfix/15369-screen-locker-buttons
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Deliverable for:

Description

  • Use title capitalization as of GNOME Style Guide.
  • Put Cancel button on the left by convention.

Files


Subtasks


Related issues

Blocks Tails - Feature #10970: Document screen locker Resolved 2016-01-18

History

#1 Updated by sajolida 2018-03-02 19:16:22

#2 Updated by sajolida 2018-03-02 19:18:32

  • Assignee changed from sajolida to segfault
  • QA Check set to Ready for QA
  • Feature Branch set to bugfix/15369-screen-locker-buttons

Here is a branch!

#3 Updated by segfault 2018-03-02 20:21:02

When we’re improving the UI, how about this? (branch bugfix/15369-screen-locker-buttons in my repo)

#4 Updated by segfault 2018-03-02 20:21:20

  • Assignee changed from segfault to sajolida
  • QA Check changed from Ready for QA to Info Needed

#5 Updated by intrigeri 2018-03-06 14:58:53

  • QA Check changed from Info Needed to Dev Needed

Please rebase this branch on testing (it’s currently based on devel) so that 1. it builds on Jenkins (Bug #15372); 2. we can plausibly merge it for 3.6 :)

#6 Updated by sajolida 2018-03-08 11:03:30

  • Assignee changed from sajolida to bertagaz
  • QA Check changed from Dev Needed to Ready for QA
  • I like the change of the title into “Screen Locker”.
  • I added a period at the end of the explanation as it’s status changed from title to the body of the dialog.
  • I prefer to be extra explicit and say “Lock Screen” in the button. I see no problem with being that explicit :)
  • I rebased the branch on testing.
  • I didn’t update the PO files because I don’t know how to do that.

#7 Updated by segfault 2018-03-08 13:01:23

sajolida wrote:
> * I added a period at the end of the explanation as it’s status changed from title to the body of the dialog.

Thanks, that’s better.

> * I prefer to be extra explicit and say “Lock Screen” in the button. I see no problem with being that explicit :)

I changed it because I thought the dialog looked better with both buttons in the headerbar being the same width (in English). It looked awful before I moved the long title to a new label outside the headerbar, but in the current version, I think it doesn’t look to bad with the “Lock Screen” button being a bit wide (see screenshot), so I’m OK with that.

> * I rebased the branch on testing.

Thanks, I forgot that.

> * I didn’t update the PO files because I don’t know how to do that.

The PO files are updated automatically when refresh-translations is executed, I’m not sure if we should do this in this branch or if it is part of the release process.

#8 Updated by segfault 2018-03-08 17:34:28

  • Status changed from Confirmed to In Progress

Applied in changeset commit:8bd132cadd71f7f836d9732e69669ca2a5992c67.

#9 Updated by intrigeri 2018-03-09 08:20:18

> The PO files are updated automatically when refresh-translations is executed, I’m not sure if we should do this in this branch or if it is part of the release process.

In general a branch with outdated PO files will fail to build reproducibly, which makes it hard(er) for the reviewer to gain confidence that merging it won’t break reproducibility of our release branch, which is a bit annoying for changes proposed so late in the freeze (arguably UI design should be done before we get a branch merged in the first place but we have no rules about that so really I’m not blaming anyone). So I would recommend updating PO files on the topic branch :)

#10 Updated by segfault 2018-03-09 08:48:56

intrigeri wrote:
> > The PO files are updated automatically when refresh-translations is executed, I’m not sure if we should do this in this branch or if it is part of the release process.
>
> In general a branch with outdated PO files will fail to build reproducibly,

I remember that you told me last week that this happened when Jenkins built the Feature #5684 and Feature #11753 branches, but now that I think about it, I don’t understand why this breaks reproducibility. If the branch is built multiple times with the same outdated PO files, it should still be reproducible, no? Can you point out what I got wrong?

> which makes it hard(er) for the reviewer to gain confidence that merging it won’t break reproducibility of our release branch, which is a bit annoying for changes proposed so late in the freeze (arguably UI design should be done before we get a branch merged in the first place but we have no rules about that so really I’m not blaming anyone). So I would recommend updating PO files on the topic branch :)

ACK, did that in 1105dbdf14662436a850c7935fa39beb1ab3f4bb.

#11 Updated by segfault 2018-03-09 08:54:57

segfault wrote:
> intrigeri wrote:
> > > The PO files are updated automatically when refresh-translations is executed, I’m not sure if we should do this in this branch or if it is part of the release process.
> >
> > In general a branch with outdated PO files will fail to build reproducibly,
>
> I remember that you told me last week that this happened when Jenkins built the Feature #5684 and Feature #11753 branches, but now that I think about it, I don’t understand why this breaks reproducibility. If the branch is built multiple times with the same outdated PO files, it should still be reproducible, no? Can you point out what I got wrong?

After looking at the diff of my commit, I suspect it’s the POT-Creation-Date which differs, which would mean that refresh-translations is run automatically during the build (is that the case?).

#12 Updated by bertagaz 2018-03-09 10:22:07

segfault wrote:
> After looking at the diff of my commit, I suspect it’s the POT-Creation-Date which differs, which would mean that refresh-translations is run automatically during the build (is that the case?).

Yes, you got it, that’s exactly why outdated PO files breaks reproducibility.

#13 Updated by bertagaz 2018-03-11 17:23:52

  • Status changed from In Progress to Fix committed
  • % Done changed from 0 to 100

Applied in changeset commit:414a14a7f4e17e2b87d6e639ec25f227cb5981b7.

#14 Updated by bertagaz 2018-03-11 17:27:01

  • Assignee deleted (bertagaz)
  • QA Check changed from Ready for QA to Pass

Sounds and looks good, merged!

#15 Updated by bertagaz 2018-03-14 11:04:14

  • Status changed from Fix committed to Resolved