Skip to content

Adding the reading time on articles' main screen#492

Closed
Simounet wants to merge 5 commits into
wallabag:masterfrom
Simounet:feature/reading-time-main-screen
Closed

Adding the reading time on articles' main screen#492
Simounet wants to merge 5 commits into
wallabag:masterfrom
Simounet:feature/reading-time-main-screen

Conversation

@Simounet
Copy link
Copy Markdown
Member

Hello there,
I wanted to add the reading time to each article on the main screen. I thinks it could be useful to know this information before switch to the article's view.
I started to do that but can't figure out how to update the placeholder with the correct value.
Also, I have to handle the right position for this information.
Any help appreciated.

@di72nn
Copy link
Copy Markdown
Member

di72nn commented Mar 30, 2017

That'll need #475 (with some additions) to update the values when the reading speed is changed in the settings.

@Simounet
Copy link
Copy Markdown
Member Author

Ok, thanks for your reply. I'll wait then. Do you think this is interesting anyway?

@di72nn
Copy link
Copy Markdown
Member

di72nn commented Mar 31, 2017

Yeah, the main lists should include more information. There might be an issue about it.

@Simounet
Copy link
Copy Markdown
Member Author

Ok so I'll wait for the other PR being solved. Ping me when it is ok to resume this one.

@Simounet Simounet force-pushed the feature/reading-time-main-screen branch from 57a0c1a to 0a970aa Compare May 18, 2017 20:44
@di72nn di72nn changed the title [WIP] Adding the reading time on articles' main screen Adding the reading time on articles' main screen May 30, 2017
@di72nn
Copy link
Copy Markdown
Member

di72nn commented May 30, 2017

The code should be more or less ok, but the UI may be changed. Maybe switch places of the time and the icons (so that the time is not shifted by icons)?

@Simounet
Copy link
Copy Markdown
Member Author

Yep, it was a POC to see how to do that. I'll try to make it better now.

@nicosomb
Copy link
Copy Markdown
Member

Is that possible to have a screenshot please? :)

@Simounet
Copy link
Copy Markdown
Member Author

wallabag-android-reading-time

@nicosomb
Copy link
Copy Markdown
Member

@di72nn is right.
Don't forget that you have icons for archived and favorites sometimes.

Maybe we can move the reading time near to the domain.

@Simounet
Copy link
Copy Markdown
Member Author

I know, WIP/POC etc. ;)

@di72nn
Copy link
Copy Markdown
Member

di72nn commented Jun 8, 2017

@Simounet there's a minor merge conflict, so I created a new branch with meaningful commits from this PR. To continue working on this feature you may create a new PR for reading_time_in_lists branch.

@ngosang
Copy link
Copy Markdown
Contributor

ngosang commented Jun 10, 2017

I'm interested in this PR. Could we keep the "star" above the reading time?

@lokesh-krishna
Copy link
Copy Markdown
Contributor

Wouldn't that "star" clash with the text of the article if it is too long?
How about the time being displayed next to the domain (either before or after it), separated by a "|".

@ngosang
Copy link
Copy Markdown
Contributor

ngosang commented Jun 11, 2017

How about the time being displayed next to the domain (either before or after it), separated by a "|".

Maybe. Coud you share some screenshots with both approaches? (With star + reading time)

@di72nn
Copy link
Copy Markdown
Member

di72nn commented Jun 11, 2017

Every article has estimated reading time, so it should have a fixed position. Occasional elements (stars) can be shifted by other elements if needed.

@ngosang
Copy link
Copy Markdown
Contributor

ngosang commented Jun 11, 2017

I was thinking in one table with:

Title (left aligned) star (right aligned)
domain (left aligned) reading time (right aligned)

@Strubbl
Copy link
Copy Markdown
Contributor

Strubbl commented Jun 29, 2017

I have rebased this PR to a branch in this repository: follow-up-pr-492-simounet-feature/reading-time-main-screen
This way, we can work together

@Strubbl
Copy link
Copy Markdown
Contributor

Strubbl commented Jun 29, 2017

I propose to close this PR in favor of #592. If that was wrong please comment or directly reopen

@Strubbl Strubbl closed this Jun 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants