ReadYou icon indicating copy to clipboard operation
ReadYou copied to clipboard

feat(ui): long press on an item to show context menu

Open JunkFood02 opened this issue 1 year ago • 2 comments

A workaround or add-on before we implement #585 Close #611

To Do:

  • [ ] Mark above/below as unread
  • [x] Enter/exit transition for dropdown menu (optional)

JunkFood02 avatar Feb 12 '24 08:02 JunkFood02

Just tested, looks stable. I'd change a few things:

  • highlight the article that opened the context menu*.
  • change the background color of the context menu to match the color of the highlighted article or some other accent color, as the current one is rather bland**.

But... maybe make it a bit easier? Place read/star/share in a grid, as these actions should be more familiar to users, and have the mark above/below as read as it is now. Something like the context menu in Chrome:

5f59f953-dc2b-4ebd-a3e2-c0db7d64b4e0

Note the divider style, which could also be applied (less visible, more subtle). This way the menu would be much more handy.

* By highlighting the item card, I mean the state when you hold your finger on it, which triggers the menu. But now, when you stop holding it, the bg color disappears - my idea is to make this display until the context menu is closed.

https://github.com/Ashinch/ReadYou/assets/110673332/9e435031-04f6-4fac-b1c1-2d50d17430b9

(Recorded it yesterday with an older build, but still applies)

** Could be the same as the triggered article tint

I also find that this shadow border does not match the overall design of the app. Maybe make the background color of the context menu the same as the background color of the app, and give the context menu a subtle outline with a tint of the color of the highlighted item? Hmm... I imagine that would be a more appealing way to handle it.

Great work as always!

nvllz avatar Feb 13 '24 10:02 nvllz

By highlighting the item card, I mean the state when you hold your finger on it, which triggers the menu. But now, when you stop holding it, the bg color disappears - my idea is to make this display until the context menu is closed.

It's a bug and I just fixed it

highlight the article that opened the context menu*.

Not sure about whether I have the energy to make another design for it, but I'll give it a try when I have time.

But... maybe make it a bit easier? Place read/star/share in a grid

No, just follow the guidelines

shadows are ugly

Yes, blame google

JunkFood02 avatar Feb 13 '24 10:02 JunkFood02

Mark above/below as read

After evaluating the existing code base and the current model of how we handle read status updating, I find it difficult to implement new logic to make the markAsRead method(s) accept an extra after: Date.

Problems

  • APIs are all written to mark as read before a certain date, modifying such APIs needs sigfinicant refactoring to the current Repositories, DAOs, and even remote API services
  • SQL queries are being used heavily for filtering, searching, and presenting the paged data to the UI layer. Writing new SQL queries, or adding new fields on existing queries is painful.

Proposed Solution

In this PR, two new methods are added to the code base, updateReadStatusByIdSet in AbstractRssRepository, and markAsReadFromListByDate in FlowViewModel.

  • updateReadStatusByIdSet

Just what its name suggests, bulk updating the read status by a Set of article id

  • markAsReadFromListByDate

Filter the list before or after the given Date, map it to the set of article id, then call updateReadStatusByIdSet

Considerations

  • Ease of implementation: no structural changes made to code base, ~60 LoC
  • Consistent with UI: Users are only presented with limited data, operations on the same paging data list won't bring unexpected behaviors, whether it's being filtered or/and being searched
  • The numbers of articles to update: "Mark above as read" should only update limited articles when a user is scrolling down the flow list. "Mark below as read" doesn't seem practical but preserved for consistency

To-Do

  • Bulk updating for FeverAPI (updateReadStatusByIdSet) cc @Ashinch

JunkFood02 avatar Mar 06 '24 06:03 JunkFood02

It seems like that fever doesn't support bulk update of the read mark of multiple items, should we disable this feature on Fever accounts to avoid sync errors? @Ashinch

JunkFood02 avatar Mar 08 '24 04:03 JunkFood02

It seems like that fever doesn't support bulk update of the read mark of multiple items, should we disable this feature on Fever accounts to avoid sync errors? @Ashinch

We can mark them as read in turn on the background threads. Can leave this todo to me.

Ashinch avatar Mar 08 '24 05:03 Ashinch

It seems like that fever doesn't support bulk update of the read mark of multiple items, should we disable this feature on Fever accounts to avoid sync errors? @Ashinch

Hi, @JunkFood02

#640 has been merged.

Usage: rssService.get().batchMarkAsRead(articleIds, false)

Ashinch avatar Mar 08 '24 10:03 Ashinch