[Pocket] Add Pocket feed view (w/ placeholders items)

[mcomella]

See #759 for progression of mocks.

We need to decide if this should be native Android (complicated back stack) or web (complicated to pass things back and forth). Our current idea is a hybrid: we use firefox:pocket to open the pocket feed view, which replaces the Browser view. This lets us leverage the browser back stack and still write things in native code.

[liuche]

@aminalhazwani, we'll need mocks for the Pocket video feed that is shown when you click on the Pocket "megatile". Thanks!

[mcomella]

You can use the leanback support lib's ListRowView to implement the carousel. Here's some sample code:

<android.support.v17.leanback.widget.ListRowView
        android:id="@+id/listRow"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"/>

To use:

class MainActivity : AppCompatActivity() {

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContentView(R.layout.activity_main)

        listRow.gridView.adapter = RowAdapter()
    }
}

class RowAdapter : RecyclerView.Adapter<RowItemViewHolder>() {

    val items = listOf(
            "test", "strings", "apple", "foo", "google", "facebook",
            "lol", "rofl", "a", "b", "c", "d", "e", "f", "g", "h"
    )

    override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): RowItemViewHolder {
        return RowItemViewHolder(LayoutInflater.from(parent.context).inflate(R.layout.row_item, parent, false))
    }

    override fun getItemCount(): Int {
        return items.size
    }

    override fun onBindViewHolder(holder: RowItemViewHolder, position: Int) {
        holder.itemView.setOnFocusChangeListener { v, hasFocus ->
            v.setBackgroundColor(Color.parseColor(if (hasFocus) "#ff0000" else "#00ff00"))
        }
        holder.textView.text = items[position]
    }
}

class RowItemViewHolder(itemView: View) : RecyclerView.ViewHolder(itemView) {
    val textView = itemView.findViewById<TextView>(R.id.text)
}

There are also higher level abstractions that combine multiple widgets (e.g. multiple rows), but this seems more flexible and simpler for our use case.

[mcomella]

I have a WIP I'm going to land. Here's a video of it in action (it's smoother on device) and a screenshot:

Note: final specs are upcoming so I didn't try to match them exactly. This is just to get the code in so others can keep hacking on it.

FYI @aminalhazwani

[mcomella]

Here's the finalized header and "Recommended videos" title:

I'll get to the card items tomorrow and we need to figure out which white we're using. CC @aminalhazwani

[mcomella]

Here's the finalized view with placeholders and no focus ring, assuming we want to use the white cards for the focused state (see this discussion for more on that):

I don't have my Fire TV to record a video but you can pull down my PR to try it in action. There are no animations when transitioning between text/background colors, but it actually looks pretty good as is.

@aminalhazwani What do you think?

My intention is to land this as (pending your feedback!), continue implementing other v3.0 features (we may be falling behind), and revisit the focus ring in https://github.com/mozilla-mobile/firefox-tv/issues/837, if we think it's more important than other polish/work.

[aminalhazwani]

@mcomella I tested your branch on the TV, it's looking good! Just a couple of minor (I guess) fixes following https://github.com/mozilla-mobile/firefox-tv/issues/759#issuecomment-383028712.

When a card is highlighted:

• cardBackground photonGrey60
• titleTextColor photonGrey10
• subdomainTextColor photonGrey10_a99
• fontFamily @string/font_roboto_medium

When a card is not highlighted:

• cardBackground photonGrey70
• titleTextColor photonGrey10_aCC
• subdomainTextColor photonGrey10_a99
• fontFamily @string/font_roboto_regular

Unrelated questions for my general knowledge 🙂

• I was able to test the changes suggested above by editing PocketVideoFragment.kt but I wasn't able to set the font family, I found and tried different thigs like setTypeface and titleTextAppearance without success, how would you implement it?
• I don't find the opacity value set in hex to be very intuitive, I am curios to learn why you opted for photonGrey10_a99 instead of photonGrey10_a60, thanks!

[mcomella]

Here it is with the requested adjustments:

@aminalhazwani Dynamically switching the font style looks a little weird - what do you think? Here's a video: https://drive.google.com/file/d/1d2H606u9W4WsHlGKoDLaDGQxiac1CAEE/view?usp=sharing We can fix this in a follow-up if you want to (because I'll probably land today).

By the way, that was a really helpful way of providing colors: thank you! :)

I was able to test the changes suggested above by editing PocketVideoFragment.kt but I wasn't able to set the font family, I found and tried different thigs like setTypeface and titleTextAppearance without success, how would you implement it?

Looking at StackOverflow, it looks like there's no simple way to do this in code (and thus change the styles dynamically). I ended up:

• Defining two TextAppearance's, one for RobotoRegular and one for RobotoMedium, in styles.xml
• Switching between the styles in the code based on the state and calling setTextAppearance

Here's my commit where I did that: https://github.com/mozilla-mobile/firefox-tv/pull/813/commits/791802fcae991a022fb6a06853cc6af55e9137a5#diff-4e5dda5abad02699cfb42cf18ef235baR96

I'm not sure how easy it is to find framework defined fonts in code to use setTypeface so it seemed easier to use the fonts from the XML.

I don't find the opacity value set in hex to be very intuitive, I am curios to learn why you opted for photonGrey10_a99 instead of photonGrey10_a60, thanks!

I don't find it intuitive either and I honestly didn't think to convert it something more intuitive. I added a commit to call them photonGrey10_a60p and similar, where p is for percent. Thanks for the suggestion. :) I think what's most important, however, is for us to have a common language so if you want to call it something else, let me know.

[mcomella]

We have the strings from the UX deliverable bug: https://github.com/mozilla-mobile/firefox-tv/issues/759#issuecomment-381996370 So I'm removing the strings-needed label. I assume Amin has followed up with Brian that this is final copy.

[aminalhazwani]

Dynamically switching the font style looks a little weird - what do you think?

@mcomella I agree, good call! Let's not switch the font weight.

Some other small things I noticed - that can be part of a later polish:

• the video thumb has squared corners at the bottom even when the card is not highlighted. Would it be possible to make all four corners rounded when the card is not highlighted?
• from what I can see the card scales with the same ratio in width and height. Would it be possible to scale width and height at different ratios?

I don't find it intuitive either

This is how we currently name them on the design system, we simply add the suffix _aXX as per the alpha value, we assume that is understandable enough while ringing enough bells https://github.com/FirefoxUX/design-tokens/blob/master/photon-colors/photon-colors.android.xml#L23

[aminalhazwani]

Dynamically switching the font style looks a little weird - what do you think?

I was re-watching the video. From what I can tell I think it looks weird because there is no transition during the font switch. Probably because we are switching the font and not simply animating the weight.

[mcomella]

looks weird because there is no transition during the font switch. Probably because we are switching the font and not simply animating the weight.

Do you mean basically growing the weight integer value, e.g. from 10 to 20? I don't think there's a way to do this in Android: afaict, you can only set different fonts and use multiple layouts to hack together some transition animation. That being said, since changing the weight can cause reflow (unless we're careful about horizontal spacing, but I'm also not sure Android gives you that control) so I don't know that these animations would look very good either.

@aminalhazwani Can you be more specific about what you had in mind? For now I'll remove the font weight change but if you could elaborate more so we can address this later in the sprint, that'd be great!

[mcomella]

the video thumb has squared corners at the bottom even when the card is not highlighted. Would it be possible to make all four corners rounded when the card is not highlighted?

This might take effort: I filed https://github.com/mozilla-mobile/firefox-tv/issues/844.

from what I can see the card scales with the same ratio in width and height. Would it be possible to scale width and height at different ratios?

Yes. We're currently scaling at by .05% when focused (in animator/pocket_video_item.xml). What should the scale values be?

[aminalhazwani]

I don't think there's a way to do this in Android: afaict, you can only set different fonts and use multiple layouts to hack together some transition animation.

Can you be more specific about what you had in mind?

@mcomella gotcha, I was exactly thinking if on Android there's a way to animate/transition only the font weight property and not the whole font, I was trying to understand if it might be possible by reading this. I'll more into how other TV apps do highlight typography in order to see if there is anything interesting that we could use too.

For now I'll remove the font weight change

👍

Yes. We're currently scaling at by .05% when focused (in animator/pocket_video_item.xml). What should the scale values be?

If I read the code correctly the non-highlighted card is 240dp large and 290dp high. We can scale the width by .1% so that it becomes 264dp while the height can remain the same. We only need to move the highlighted card up - y axis - by 3.5dp if possible, so that the thumb of an highlighted card is vertically aligned with the thumb of a non-highlighted card.

As mentioned before this is low priority and can be addressed in a later polish sprint.

[mcomella]

I was trying to understand if it might be possible by reading this.

TextAttribute appears to be an immutable field to describe font metadata (e.g. one instance would be "FONT_WEIGHT" with a solid value of "5"). I can't figure out anyway this is used by the framework so it might only be used internally. Given that there is no "font_weight" property in XML and to change the weight, you have to specify a whole different font, I'm hesitant that there'd be a simple way to do this but it might be possible.

wrt scaling, I filed https://github.com/mozilla-mobile/firefox-tv/issues/855

[aminalhazwani]

Cool, thanks @mcomella!