Closed Bug 1391613 Opened 2 years ago Closed 2 years ago

Update style for hover and keyboard selection in URL bar to be same as hamburger menu

Categories

(Firefox :: Theme, enhancement, P1)

57 Branch
enhancement

Tracking

()

RESOLVED FIXED
Firefox 57
Iteration:
57.3 - Sep 19
Tracking Status
firefox57 --- fixed

People

(Reporter: afnankhan, Assigned: dao)

References

(Blocks 1 open bug)

Details

(Whiteboard: [reserve-photon-visual])

Attachments

(2 files)

Attached image Hamburger menu
No description provided.
Whiteboard: [photon-visual][triage]
Priority: -- → P4
Whiteboard: [photon-visual][triage] → [reserve-photon-visual]
See Also: → 1392582
I'm not sure we want the exact same style here, but we should probably remove the border from the hover state.
Assignee: nobody → dao+bmo
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Priority: P4 → P1
Attachment #8900539 - Flags: ui-review?(shorlander)
Iteration: --- → 57.2 - Aug 29
Flags: qe-verify?
Flags: qe-verify? → qe-verify-
Where'd this bug come from?  I haven't heard anything about changing the urlbar popup styling, or removing the border on its items, and there doesn't seem to be any mention of the popup in the Invision spec.
(In reply to Drew Willcoxon :adw from comment #3)
> Where'd this bug come from?

From a user observing an inconsistency. This used to more or less match the hover feedback in Australis panels. Now that we're changing these, it makes sense to update this too.
I don't think that follows, but could you please check with Stephen first?
Comment on attachment 8900539 [details]
Bug 1391613 - Remove border from URL bar and search bar autocomplete items. ui-r=shorlander

https://reviewboard.mozilla.org/r/171936/#review177674

::: browser/themes/shared/urlbar-autocomplete.inc.css:12
(Diff revision 1)
>  #treecolAutoCompleteImage {
>    max-width: 36px;
>  }
>  
>  .autocomplete-richlistbox {
> -  padding: 4px;
> +  padding: 4px 3px;

Why this change?
Attachment #8900539 - Flags: review?(adw)
I didn't mean to leave comment 6 when I cleared the review, so please ignore it for now.
(In reply to Drew Willcoxon :adw from comment #5)
> I don't think that follows,

What doesn't follow?

> but could you please check with Stephen first?

The patch is already waiting for Stephen's ui-review.
(In reply to Dão Gottwald [::dao] from comment #4)
> This used to more or less match the hover feedback in Australis panels.
> Now that we're changing these, it makes sense to update this too.

I don't think it follows that because we're changing hover feedback in panels that it makes sense to update hover feedback in the urlbar popup.  The urlbar popup is one of the most important pieces of UI.  Surely if UX had intended it to be updated, they would have included it in the Invision spec, and by now I would know about it.

But I could be wrong.  Please re-request review once Stephen says that the urlbar popup should be changed in this way.
I don't think it's useful to muse about why there's no mockup for this and whether or not somebody had this on their radar. Let's evaluate this change on its own merits, which would be consistency across the UI.
Comment on attachment 8900539 [details]
Bug 1391613 - Remove border from URL bar and search bar autocomplete items. ui-r=shorlander

We overlooked the awesome bar results in the initial Photon spec. Aligning the various states with the other Photon styles is the right thing to do. The try-builds looked good to me. Thank you!
Attachment #8900539 - Flags: ui-review?(shorlander) → ui-review+
Iteration: 57.2 - Aug 29 → 57.3 - Sep 19
Comment on attachment 8900539 [details]
Bug 1391613 - Remove border from URL bar and search bar autocomplete items. ui-r=shorlander

https://reviewboard.mozilla.org/r/171936/#review179306
Attachment #8900539 - Flags: review+
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/781da96f5a62
Remove border from URL bar and search bar autocomplete items. ui-r=shorlander r=adw
https://hg.mozilla.org/mozilla-central/rev/781da96f5a62
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
You need to log in before you can comment on or make changes to this bug.