Closed Bug 835842 Opened 7 years ago Closed 6 years ago

[Calendar UX VD] Use BB for Tabs, the have wrong hit state and odd style

Categories

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

x86
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: vicky, Assigned: pivanov)

References

Details

(Whiteboard: visual design, UX P2, [TEF_REQ], PRODUCT-CONSISTENCY, visual-tracking)

Attachments

(3 files)

Tabs have a light grey/violet hit state right now, where it should have the consistent light blue state.

Apply building blocks to this component.
Whiteboard: visual design, UX P2, [TEF_REQ], PRODUCT-CONSISTENCY
Duplicate of this bug: 818116
Blocks: 818486
Can you provide a link to the appropriate building block? Sorry, I'm relatively new to the project so I'm not even quite sure what the issue is without having the spec to compare against.
Darius, find here the building block you need to apply:
http://buildingfirefoxos.com/index.html#filters
If you use Nighty to browse this website you´ll be able to inspect the example, in other browsers it uses just an image.
Pavel, can you apply these building blocks for tabs? Thanks!
http://buildingfirefoxos.com/building-blocks/filters/
Assignee: nobody → pivanov
Flags: needinfo?(pivanov)
Whiteboard: visual design, UX P2, [TEF_REQ], PRODUCT-CONSISTENCY → visual design, UX P2, [TEF_REQ], PRODUCT-CONSISTENCY, visual-tracking
Attached file patch for Gaia repo
Flags: needinfo?(pivanov)
Attached image After patch
Attachment #754270 - Flags: feedback?(epang)
Comment on attachment 754270 [details]
After patch

This looks great, Thanks Pavel.  It's ready for review!
Attachment #754270 - Flags: feedback?(epang) → feedback+
Attachment #754269 - Flags: review?(sjochimek)
Comment on attachment 754269 [details]
patch for Gaia repo

Pavel: Can you update the PR, seems to have conflicts ? (flag me back)
Attachment #754269 - Flags: review?(sjochimek)
Flags: needinfo?(pivanov)
Sure ... it's ready for review
Flags: needinfo?(pivanov)
Sam, this is ready for review.
Flags: needinfo?(sjochimek)
Attachment #754269 - Flags: review?(sjochimek)
Comment on attachment 754269 [details]
patch for Gaia repo

I have a question, BB is for general purpose. 
Why use BB selector in the context of the app ?
I really think you can keep #view-selector to customize BB.
Attachment #754269 - Flags: review?(sjochimek)
Attachment #754269 - Flags: review?(sjochimek)
Comment on attachment 754269 [details]
patch for Gaia repo

Pavel: few remarks:
1. I don't understand why do you need to target the '.today'. I tried it without all the 3 .today selectors and it still works;
2. Can you avoid selectors using data-*, you can use `#day-view.active ~` for instance instead.

Thanks

(flag me back)
Attachment #754269 - Flags: review?(sjochimek)
Flags: needinfo?(sjochimek)
Comment on attachment 754269 [details]
patch for Gaia repo

Thanks Sam, I saw more things that I can fix (sorry for this forgetfulness).
Attachment #754269 - Flags: review?(sjochimek)
Pavel: What about the point 2. ?
Flags: needinfo?(pivanov)
Attachment #754269 - Flags: review?(sjochimek)
Hey Sam,
thanks for the question. I had two reasons to discard point 2.

The first one is:
I'm not sure whether it is better to bind selectors with "~" because it would be too dependent on the markup of this.

The second one is:
The CSS specificity of this is too hard. If I understand correctly the selectors will be:

with [data-*] : body[data-path="/month/"] #view-selector > .month > a {}
with "~"      : #week-view.active #view-selector > .month > a {}

and the second one is more specific(slow)

Hope this explain my concerns.
Flags: needinfo?(pivanov) → needinfo?(sjochimek)
Comment on attachment 754269 [details]
patch for Gaia repo

I was trying to optimize css selector but after few tests that doesn't change anything. So it's r+
Attachment #754269 - Flags: review+
Naoki, this should be ready for verification. Thanks!
Flags: needinfo?(nhirata.bugzilla)
FYI, when I tried to merge this change with my repo from master, I ran into a conflict : 

git pull https://github.com/pivanov/gaia.git bug-835842

Performing inexact rename detection: 100% (166901/166901), done.
Auto-merging apps/calendar/style/ui.css
CONFLICT (content): Merge conflict in apps/calendar/style/ui.css
Auto-merging apps/calendar/style/day_views.css
Auto-merging apps/calendar/index.html

We may want to fix the conflict before asking to push it to MC/master
Hey Naoki,
this one was ready before one month ... but I just rebase and fix the conflicts so ... now it's ready for verification :)
I get a make: *** [webapp-zip] Error 3 with issues with keys and such missing after updating to this patch.  I'm not sure how to resolve it.  Prior to the patch, I had built just to make sure it was working ok; and it worked just fine.

Is there a way for me to resolve this make issue?
Flags: needinfo?(nhirata.bugzilla) → needinfo?(pivanov)
Comment on attachment 754269 [details]
patch for Gaia repo

Have the same build problem as Naoki mentioned 'make: *** [webapp-zip] Error 3'

(Just to prevent confusion, as this bug is not landed yet, i removed r+).
Attachment #754269 - Flags: review+ → review?(sjochimek)
Flags: needinfo?(sjochimek)
hm, I'm not sure how to resolve this ... but I will ask somebody and ping you back when I have something
Flags: needinfo?(pivanov)
Attachment #754269 - Flags: review?(sjochimek)
(In reply to Pavel Ivanov [:ivanovpavel] from comment #22)
> hm, I'm not sure how to resolve this ... but I will ask somebody and ping
> you back when I have something

Hey Pavel, any updates for this bug?  Would really like to get this into 1.2.
Flags: needinfo?(pivanov)
the problem was this line:
<link rel="stylesheet" type="text/css" href="/shared/style/responsive.css" >

we don't have this file anymore ... 

So it's ready to land Sam :)
Flags: needinfo?(pivanov)
Attachment #754269 - Flags: review?(sjochimek)
Is there suppose to be a hit state for the hour selected in the day view or the day/hour selection in the week view?  Currently with this patch, I do not see the blue highlight as a hit state.
Flags: needinfo?(vpg)
Comment on attachment 754269 [details]
patch for Gaia repo

landed in master: https://github.com/mozilla-b2g/gaia/commit/f240abf567325d486c3d0434791b1d132cfa327a
Attachment #754269 - Flags: review?(sjochimek) → review+
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Flags: needinfo?(vpg)
You need to log in before you can comment on or make changes to this bug.