796669 - Bring the settings button on screen a bit more and increase its size to allow for a bigger click range

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Calendar, defect)

Description

[GH to BZ]

[GitHub issue by jds2501 on 2012-09-18T00:59:50Z, https://github.com/mozilla-b2g/gaia/issues/4839]
## Build

* Device: Otoro
* Gaia: 97a730d0457c62a5811198cd9ae1f758bcfbb715
* Mozilla Central: d3bb93eead1bdd12d0596dc52a20c813287449b8

## Issue

Nit-pick - I'd bring the calendar menu option a bit more on screen and increase the click range for the button. It's a bit too small to be clicked on in the Otoro, although definitely possible to do so.

Comment 2

[Leo]

I have brought the menu button a bit more on screen and increased the click range for the button. Please find the pointer for pull request for the patch at https://github.com/mozilla-b2g/gaia/pull/8115. Please review the patch.

Comment 3

[Leo]

Attachment: Pointer to pull request

Comment 4

[Kevin Grandon :kgrandon]

I am concerned about this fix as this makes a change to only local building blocks of calendar (not sure why Calendar has a copy of them). We see in the default that it is set to a different value - so it seems like we should be changing it in both places.

We would definitely need a screenshot and to ask for UX approval here I think. leo - can you attach a screenshot and we'll get confirmation from UX? Thanks!

Comment 5

[Leo]

Attachment: Screnshot without patch

Comment 6

[Leo]

Attachment: Screenshot with patch

Please have a look at the attached screenshots.

Comment 7

[Kevin Grandon :kgrandon]

Comment on attachment 715322 [details]
Screenshot with patch

Hi Casey - I'm just trying to get some UX feedback on these changes. I think that if we should make these they should probably go into /shared and not just this single app. It seems odd to me that we should do this for just one app. Thanks!

Comment 8

[Kevin Grandon :kgrandon]

Comment on attachment 714292 [details]
Pointer to pull request

Pavel - adding you as a reviewer too as you might know the CSS landscape better than I. I'm just concerned about changing this, but not any other apps. Thanks!

Comment 9

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

The patch look simple, bug do you try to use the header building blog?

Comment 10

[Kevin Grandon :kgrandon]

@pavel - I am specifically worried about only doing this for calendar and not on a global level. If we do this for calendar the header will look different than other apps.

Comment 11

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Thats why I want to use [BB] :) if we need to make changes like this one ... we need to make changes in [BB]

Comment 12

[Casey Yee]

Kevin,
This is a super-old issue that was addressed by making the drawer button hit-area larger.  The change was applied to the BB's after the original issue was filed.  

Because of this, I don't think it's necessary to increase the size of the drawer icon if we apply the proper building block for it.

Comment 13

[Casey Yee]

Comment on attachment 715322 [details]
Screenshot with patch

See comment #12

Comment 14

[Kevin Grandon :kgrandon]

Comment on attachment 714292 [details]
Pointer to pull request

It appears that the current solution does not match up to the current building block code. Please check the current building block code, and ensure that our local copy matches. If we need to make any updates, we should make updates to both places.

I believe the current code is in: /shared/style/headers.css

Comment 15

[Kevin Grandon :kgrandon]

Comment on attachment 714292 [details]
Pointer to pull request

I think this is fine, but I would also like Pavel to review it if possible.

Comment 16

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Please See comment #12
We need to increase the hit-area, not to change the visuals. Currently we have 5rem here :
https://github.com/mozilla-b2g/gaia/blob/master/shared/style/headers.css#L222

If you need more info just ping me on IRC or here/email.

Comment 17

[Kevin Grandon :kgrandon]

Comment on attachment 714292 [details]
Pointer to pull request

What Pavel has said makes total sense. As we already have a 5rem hit-target, I'm not sure it makes sense to increase this any further? Anyway, I'm clearing the review flag for now.

If we want a larger hit area, that would be fine. Otherwise I believe that this should be driven by UX.

Comment 18

[Leo]

hmm, the increase in the hit area totally serves the purpose. 

I would like to know, if i want to make changes for UX how can i propose to UX team.

thanks

Comment 19

[Chris Lord [:cwiiis]]

The calendar drawer button is larger and has a larger hit area now since the 2.0 redesign, marking this as fixed.