[remote-dbg-next] UX-implementation: Runtime page header

RESOLVED FIXED in Firefox 67

Status

enhancement
P1
normal
RESOLVED FIXED
4 months ago
3 months ago

People

(Reporter: jdescottes, Assigned: jdescottes)

Tracking

(Blocks 1 bug)

unspecified
Firefox 67
Dependency tree / graph

Firefox Tracking Flags

(firefox67 fixed)

Details

Attachments

(7 attachments, 1 obsolete attachment)

Assignee

Description

4 months ago

Follow up to Bug 1512636. We have finalized mockups for the header of the runtime page and we should implement it.

Note that we have 2 bugs where we are adding buttons for remote runtime pages:

  • disconnect button
  • profile runtime button

We should probably wait until both those bugs have landed before taking this.

Assignee

Updated

3 months ago
Priority: -- → P2
Assignee

Updated

3 months ago
No longer depends on: 1505128
See Also: → 1505128

Updated

3 months ago
Assignee: nobody → balbeza
Status: NEW → ASSIGNED

Updated

3 months ago
Assignee: balbeza → nobody
Status: ASSIGNED → NEW
Assignee

Updated

3 months ago
Duplicate of this bug: 1527240
Assignee

Comment 3

3 months ago

From Bug 1527240

Follow up to Bug 1495665. We are starting to have more and more actions in the runtime header. Once all actions have been added we should cleanup the UI.

From :ladybenko's review in Bug 1495665:

Not for this bug, but the way we are organising our buttons / controls in the top of the runtime page feels a bit messy.
Maybe we could have a grid to lay out the different elements

Assignee

Updated

3 months ago
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Priority: P2 → P1
Assignee

Comment 4

3 months ago

Review of the gap between the content of the mockups and our current implementation

  • mockups show the Disconnect button, but it is missing (Bug 1505128)
    => will add the button as disabled for now
  • mockups only show a Runtime with a Device, but we have runtime pages without devices
    => we have one old screenshot which shows the Runtime name should be centered in this case. we can try to infer the rest of the sizings from this
  • mockups do not include the "Profile Runtime" button, which we added recently
    => this button should probably be next to "Disable/Enable Connection Prompt" rather than on the other side of the panel as it is now
  • mockups have the addons controls split between the Runtime Header and the Temporary Extension Category, while we kept them grouped together
    => to achieve this I think we'd need to refactor the Runtime Page (along the lines of something :ladybenko suggested this week). I would probably keep the addon actions as a single element in the Runtime Header for now, and spin this in a separate bug
Assignee

Comment 5

3 months ago

Small cleanup, this prop is no longer used by the component

Assignee

Updated

3 months ago
Keywords: leave-open
Assignee

Comment 6

3 months ago

@daisuke I would like to share this preliminary work with you.
I want to move this to a dedicated component anyway, but most importantly this will reduce merge conflicts between our 2 bugs.
I can land that and leave-open while I continue to work on the actual UX implementation?

Comment 7

3 months ago
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/966ad7948d5f
Stop passing unused temporaryInstallError prop to TemporaryExtensionInstaller;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/1bb0a92ee9e6
Move RuntimeActions to dedicated component;r=daisuke
Assignee

Comment 8

3 months ago

Depends on D20399

Assignee

Comment 10

3 months ago

Depends on D20439

I want to separate the addon checkbox and the load temporary addon for 2 reasons:

  • we will get remove the checkbox after Bug 1525533
  • we may support temporary addons for remote runtimes later

The "load temporary addon" might also move inside the Temporary Extensions category.

Assignee

Comment 11

3 months ago

Depends on D20464

For this change I moved all the layout logic for the RuntimeInfo h1 to RuntimeInfo.css
I don't know if we prefer to have layout info in base.css or in specialized components, but the RuntimeInfo header has a very specific structure.
I can't see how this would be worth having in base.css. And it is the only consumer for main-heading, so I removed all layout rules from base.css for main-heading.

Assignee

Comment 12

3 months ago

Depends on D20471

I can drop this changeset if we prefer, but this is how I imagine the disconnect button would fit in the RuntimeInfo.

Assignee

Comment 14

3 months ago

Depends on D20473

This changeset is optional, I think it might be better to handle it as part of the global layout UX bug
(https://bugzilla.mozilla.org/show_bug.cgi?id=1505126). But since some dimensions took some time to figure out I prefer to share the patch.

The basis for this patch is https://bug1512636.bmoattachments.org/attachment.cgi?id=9037195, from where we can see:

  • sidebar margin/padding top is 100px
  • runtime page header margin/padding top is 36px

So first difference with our current implementation is that .sidebar and .page are no horizontally aligned.
Moreover the Connect/Setup page does not use 36px.
I couldn't find a document with the exact measure, but looking at the last mockups I think the value used is 19 * 4px (76px).

The Header UX mockup also contains measurements for the sidebar's vertical margins and paddings but this shouldbe handled in the sidebar bug.
Again I am happy to drop the patch from this queue and move it to Bug 1505126 if you feel like it would be better to tackle this at the end.

Assignee

Comment 16

3 months ago

(first patches landed on central, removing leave-open)

Keywords: leave-open

Comment on attachment 9045231 [details]
Bug 1525619 - Update horizontal alignments according to mockups for Sidebar, ConnectPage, RuntimePage

Revision D20481 was moved to bug 1505126. Setting attachment 9045231 [details] to obsolete.

Attachment #9045231 - Attachment is obsolete: true

Comment 19

3 months ago
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/37e45833df18
Separate runtime action buttons from addon debug checkbox r=daisuke
https://hg.mozilla.org/integration/autoland/rev/ecf4f66d2547
Update layout for RuntimeInfo to display device name on separate line;r=ladybenko,daisuke
https://hg.mozilla.org/integration/autoland/rev/77903a61daae
Add Disconnect button as disabled in the RuntimeInfo layout r=daisuke,ladybenko
https://hg.mozilla.org/integration/autoland/rev/9eab7d33fb58
Update typography for RuntimeInfo r=ladybenko,daisuke
You need to log in before you can comment on or make changes to this bug.