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

Status: RESOLVED FIXED

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

Description

[Victoria Gerchinhoren [:vicky]]

Attachment: Shows incorrect current implementation

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.

Comment 2

[Darius K.]

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.

Comment 3

[Arnau March [:arnau] ( not working in Firefox OS anymore :( )]

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.

Comment 4

[Eric Pang [:epang] UX]

Pavel, can you apply these building blocks for tabs? Thanks!
http://buildingfirefoxos.com/building-blocks/filters/

Comment 5

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

Attachment: patch for Gaia repo

Comment 6

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

Attachment: After patch

Comment 7

[Eric Pang [:epang] UX]

Comment on attachment 754270 [details]
After patch

This looks great, Thanks Pavel.  It's ready for review!

Comment 8

[Sam Joch [:samjoch]]

Comment on attachment 754269 [details]
patch for Gaia repo

Pavel: Can you update the PR, seems to have conflicts ? (flag me back)

Comment 9

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

Sure ... it's ready for review

Comment 10

[Eric Pang [:epang] UX]

Sam, this is ready for review.

Comment 11

[Sam Joch [:samjoch]]

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.

Comment 12

[Sam Joch [:samjoch]]

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)

Comment 13

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

Comment on attachment 754269 [details]
patch for Gaia repo

Thanks Sam, I saw more things that I can fix (sorry for this forgetfulness).

Comment 14

[Sam Joch [:samjoch]]

Pavel: What about the point 2. ?

Comment 15

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

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.

Comment 16

[Sam Joch [:samjoch]]

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+

Comment 17

[Eric Pang [:epang] UX]

Naoki, this should be ready for verification. Thanks!

Comment 18

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

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

Comment 19

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

Hey Naoki,
this one was ready before one month ... but I just rebase and fix the conflicts so ... now it's ready for verification :)

Comment 20

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

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?

Comment 21

[Sam Joch [:samjoch]]

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+).

Comment 22

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

hm, I'm not sure how to resolve this ... but I will ask somebody and ping you back when I have something

Comment 23

[Eric Pang [:epang] UX]

(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.

Comment 24

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

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 :)

Comment 25

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

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.

Comment 26

[Sam Joch [:samjoch]]

Comment on attachment 754269 [details]
patch for Gaia repo

landed in master: https://github.com/mozilla-b2g/gaia/commit/f240abf567325d486c3d0434791b1d132cfa327a