Implement redesigned settings panel with edit functionality

RESOLVED FIXED in Firefox 68

Status

enhancement
P3
normal
RESOLVED FIXED
11 months ago
4 days ago

People

(Reporter: victoria, Assigned: mtigley)

Tracking

(Blocks 2 bugs, Regressed 1 bug)

unspecified
Firefox 68
Dependency tree / graph

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Mockup deck (6 screens): https://mozilla.invisionapp.com/share/YVNU09I73WN
Sample Inspect link for development (requires login): https://mozilla.invisionapp.com/d/main#/console/15276042/317592695/inspect

Thanks to our contributor, Kristin, for her awesome work on these mockups! (https://github.com/devtools-html/ux/issues/23)
Priority: -- → P3
Summary: Responsive Design Mode — Implement redesigned settings panel with edit functionality → Implement redesigned settings panel with edit functionality
Duplicate of this bug: 1498501
See Also: → 1498536
Assignee: nobody → mtigley
Status: NEW → ASSIGNED
See Also: → 1394996
It would be a good idea to break this down into two parts: 

Part 1: Implementing the redesign (slides 1-4) since it only concerns styling the existing settings view.

Part 2: Adding the ability to edit device names/parameters (slides 5-6). It seems we don't have a way to update a custom device directly so I think it would make sense to have it in its own part. Initial thoughts: add a new action called updateCustomDevice which would update both the store and the custom device from local storage once the "Update" button is pressed.
This is part 1 of implementing the redesigned device settings panel. In this patch we are rearranging the existing device settings view to match the new design.

Hey Patrick, I wanted to get your input on whether or not this might be a good candidate to consider using mult-column layout over grid (see Part 1 patch) to improve on the responsiveness of the modal and variable number of devices.

Flags: needinfo?(pbrosset)

I think it's perfectly valid to use css grid here. It allows for responsiveness just as well, if not better, than css mulicol.
In order to improve the responsiveness of the modal, I think we should add a few media-query breakpoints to drop down to 2 and then 1 column, as space reduces.

A proposal:

1000px:

phone phone custom
tablet laptop tv

600px

phone phone
tablet laptop
tv custom

< 600px

phone
phone
tablet
laptop
tv
custom

Using grid-template-areas for this would be awesome! You could just use these name right there in the .device-modal-content rule (and redefine the areas in the media queries), and then just use grid-area in the different .device-type-phones, .device-type-custom, etc. rules.

At the same time, it might help making the modal itself. It has a 800px max-width, which is fine to avoid it looking weird on extra large screens. But then it has a 60% width (60% of the browser window's width) which makes it look really tiny when the window becomes small. I would increase this 90%. It won't change anything in most situations (because of the max-width), so the original design is preserved. But it'll look way better below 800px.

Flags: needinfo?(pbrosset)
Blocks: 1498492
Blocks: 1493094
No longer blocks: 1394996
Duplicate of this bug: 1394996
Posted file Part 2: Add ability to edit devices (obsolete) —

I think we might want to land Part 1 after the merge since the release candidate will be made this immediate Monday.

(In reply to Gabriel [:gl] (ΦωΦ) from comment #8)

I think we might want to land Part 1 after the merge since the release candidate will be made this immediate Monday.

This sounds good to me. Landing after merge will give us more time to test the new design and device editing.

Pushed by mtigley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fbdd97d8e41d
Part I: Rearranging devices setting view to new design. r=gl,flod
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68

Quick note: since one patch landed here and the bug wasn't marked as "leave-open", it got marked as RESOLVED/FIXED in 68.
However part 2 patch hasn't landed.

It's a good idea to use the "leave-open" keyword when we expect to be landing patches at different times, so bugs don't get closed in between.
It's however sometimes a bit dangerous because it sometimes happen that we land 1 part, but never anything else, and the bug never gets marked as closed because of that.
In this particular case, part 2 seems ready, so it might be ok to mark this bug as re-open and land the part 2.
But I think it might be just as easy to file a new one just for the device editing capability, and move the patch there (as we do that, we should also make sure this new bug blocks bug rdm-ux and bug 1493094 as this one does).

Micah: do you want to do that?

Flags: needinfo?(mtigley)

Patrick: Let's create a new bug for part 2 and land it there.

Flags: needinfo?(mtigley)

Comment on attachment 9048655 [details]
Part 2: Add ability to edit devices

Revision D22180 was moved to bug 1536808. Setting attachment 9048655 [details] to obsolete.

Attachment #9048655 - Attachment is obsolete: true
See Also: → 1325571
Duplicate of this bug: 1266416

Just want to make sure what I need for 68 vs. 69. For 68, only the UI update, correct?

Flags: needinfo?(mtigley)

(In reply to Irene Smith from comment #17)

Just want to make sure what I need for 68 vs. 69. For 68, only the UI update, correct?

Hi Irene, for this specific bug yes this is only a UI update. But we also added the ability to edit devices in bug 1536808.

Flags: needinfo?(mtigley)
You need to log in before you can comment on or make changes to this bug.