Page MenuHomePhabricator

Add activity for lists that contain X article + list links
Closed, ResolvedPublic2 Estimated Story Points

Authored By
Charlotte
Jan 23 2019, 6:19 PM
Referenced Files
F28348894: android-styles.zip
Mar 8 2019, 8:01 PM
F28311173: Screenshot_20190301-145840.jpg
Mar 4 2019, 6:35 PM
F28311177: Screenshot_20190301-145617.jpg
Mar 4 2019, 6:35 PM
F28311175: Screenshot_20190301-145454.jpg
Mar 4 2019, 6:35 PM
F28016713: My lists search 04.png
Jan 24 2019, 4:21 PM
F28014406: My lists search 08.png
Jan 24 2019, 11:54 AM
F28013731: My lists search 04.png
Jan 24 2019, 11:33 AM
F28013738: My lists search 07.png
Jan 24 2019, 11:33 AM

Description

Design

👉 Search in “My lists“ | T214504 on Zeplin

Flow

https://overflow.io/s/QEVP3O/?node=72a041d1

Key aspects

  • Articles in search results are enhanced with information about the lists in which they’re saved in. See chips, e.g. “Cities“ or “People“ in the mocks.
  • Tapping a chip on an article item takes users to the corresponding list of the article
  • Chips are horizontally scrollable if they don’t fit the screen width (e.g. when an article is saved to a lot of lists or list names are long)

Event Timeline

Charlotte triaged this task as Medium priority.
Charlotte renamed this task from Add activity for lists that contain X article to Add activity for lists that contain X article + blue link.Jan 23 2019, 6:23 PM
Charlotte changed the point value for this task from 5 to 2.
This comment was removed by scblr.
scblr renamed this task from Add activity for lists that contain X article + blue link to Add activity for lists that contain X article + list links.Feb 11 2019, 5:47 PM

@schoenbaechler Could you please update the designs to reflect what should happen when page items have multiline descriptions?

Hey @Sharvaniharan: I added an additional screen to the section that shows how this is handled: https://zpl.io/aw4qMBJ. Ideally, an article description should not take up more than 2 lines. I suggest to truncate text that is longer than 2 lines with an ellipsis. Let me know in case there’s already another solution applied at some other place in the app (which could be reused). Thanks!

@schoenbaechler - Do we need the parentheses () around the ellipsis? Looks a bit more natural if the ellipsis is appended to the end of the last word...

:D

@Charlotte, after diving into typographical rules for ellipses for half an hour, I totally agree with you. It’s more clean. Thanks for your input and keen eye! 🎯 Updated the screens...

Great work @Sharvaniharan. Some things I mentioned in T214505 Show lists + list items in same list based on search term are relevant for this task too. In short:

  • The colors of the chips in the various themes are not adapted yet.
SepiaDarkBlack
Screenshot_20190301-145840.jpg (1,080×2,280 px, 313 KB)
Screenshot_20190301-145454.jpg (1,080×2,280 px, 296 KB)
Screenshot_20190301-145617.jpg (1,080×2,280 px, 290 KB)

Please translate colors to the other themes according to the Android Theme guidelines.

  • Thumbnail width should be 56x56px
  • Scrolling for the chips should end where the title & description starts, see designs on Zeplin
  • Search within a list (e.g. Saved) should work exactly the same as the global “My lists“ search. It currently does not output the chips and cuts of the status bar on my device.
  • Minor thing: I noticed there’s a blue shadow on the left and right edge of the chips when scrolling horizontally (not the Material design tap indicator when tapping a chip itself). Could we remove that or is that kind of a standard behavior? It doesn’t add too much value there. Here’s a screen recording that shows the effect.

Moving it back to “Did not pass QA“. Thanks!

Thumbnail width should be 56x56px

If we change the size of this thumbnail, it will impact the other places where this component appears, e.g. History items, Search results, Feed list items, etc. Do we want to proceed with this?

Hey @Dbrant, is there a way to apply the 56x56px width for article thumbnails just within “My lists“ ? It’s currently the only place where we’re mixing article and list results. When the indentation does not differ for article and list items in the search, it’ll be easier to scan and it looks more balanced.

I added this comment to T214506 first, but @cooltey mentioned it belongs here:

After updating reading lists, as shown in this video, the search results view in does not represent the changes. In this case, “Saved“ is still listed as a chip. Ideally, the view would update to not confuse users.

Perfectly implemented, @Sharvaniharan.

I noticed though that the chips in dark and black mode don’t work well with the current colors.
Please adapt the colors of the chips as follows:

font-colorbackground-color
Dark theme#EAECF0 (Base80)#54595D (Base20)
Black theme#C8CCD1 (Base70)#2E3136 (Base14)

Thanks!

Thank you @schoenbaechler :)
So the background combination now would be

ElementLightDarkBlack
Chip Backgroundbase80base20base14
Chip text colorbase20base80base70

I don't see these combinations on our theme guide : https://www.mediawiki.org/wiki/Wikimedia_Apps/Android_theme_guidelines#Android_theme_color_usage

Also brings up a tiny practice we can follow going forward to avoid confusion. When we see only the light-mode color in zeplin, it is a bit confusing which color attribute to use. So going forward, if it is ok with you, I suggest we do two things:

  1. Synchronize the theme page with the attribute names used in the app . You can use the files attached to use attribute names [in attrs.xml] consistent with the app. If the attribute doesn't have an entry in styles_black.xml file, it falls back to styles_dark.xml entry and if it is not found in styles_sepia.xml file, it falls back to styles_light.xml entry.

  1. Going forward, if you can give us an element name for the color, like 'material_theme_primary_color', 'chart_shade1' etc., instead of or along with the light mode color used in the zeplin component, it would be perfect and would take out the guesswork and make is so much more easy for us [ not to mention it will save you a few rounds of review 😂]. You can add the color name as a note on the zeplin screen or a commented out line on the component xml part, or any other way you feel is better. :)
  1. Anytime you want to introduce a new color attribute, first make an entry in the theme guide with respective values for all modes, and just refer to it in zeplin and we will add that new attribute to our styles and use it.

I think #3 is what we need for this case, although for urgency of current release cycle, if you can just mention a new attribute name for these two, I can update the colors this time.]

CCing @Dbrant and @cooltey - Thoughts and inputs welcome.

Hi @Sharvaniharan, thanks for the thorough feedback. I’m happy that you brought this up. A component based color breakdown makes sense to me. To move forward for now:

(...) if you can just mention a new attribute name for these two, I can update the colors this time.

Attribute nameLightSepiaDarkBlack
chip_backgroundbase80amatebase20base14
chip_textbase20masibase80base70

... and in regards to:

I don't see these combinations on our theme guide

You are right, that combination does not exist yet. The colors listed above are a result of exploring various color combinations. They base on colors that are already in use and suit better for this particular use case.


Some more thoughts:

Using the word chip in these names is obviously not very scalable. I’m thinking out loud here – would it make sense to name these color groups in a generic way, such as color_group_1? A breakdown of it could look like this:

Color group nameLightSepiaDarkBlack
'color_group_1`base80amatebase20base14
'color_group_2`base20masibase80base70

Furthermore, all colors with corresponding values could be listed in a separate table, e.g.

Color namehexrgba
base14#2e3136
base20#54595d
base70#54595d
base80#eaecf0
black54rgba(0, 0, 0, 0.54);
black87rgba(0, 0, 0, 0.87);

Let me know what you think @Sharvaniharan.

@schoenbaechler according to our conversation, I have referenced the new color groups as 'chip_background_color' and 'chip_text_color'.

With the new guidelines page, this was a pleasure to review @Sharvaniharan. Moving it to “Ready for signoff“. Thanks!

FYI: Added chip_text_color and chip_background_color to Android theme guidelines.