Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Vue Rewrite] Starred component #2321

Merged
merged 9 commits into from
Aug 22, 2023

Conversation

devlinjunker
Copy link
Contributor

@devlinjunker devlinjunker commented Aug 16, 2023

Related to #748

Follows #2306

🗒️ Summary

Started looking into how the Bookmarks app was displaying lists and a three-panel-layout and started using some code from there, but realized I should just mimic a basic use case of the current app so that it's not too large of a change and is understandable for others to review

I decided to focus on making the first iteration of the rewrite similar to the current app compact view + mark as read behavior on open (rather than scroll), as that is how I currently use the News app...

  • Added VirtualScroll Component from Bookmarks App
    • each Feed Items is created as a different #default template inside of the VirtualScroll parent component, the VirtualScroll determines which ones to render and when to load more items, or the feed has reached the end of loading
    • I still need to fully explore how the :reached-end property and load-more attributes should work
    • I still need to think about unit tests here... based on the Vue documentation, my unit tests aren't really focusing on the right things... so the unit tests need more work in general here... I'm trying to maintain a standard without slowing down development too much...
  • Created FeedItem component for displaying News Items
    • Copied (most of) HTML from original app and attempted to match styles
    • Added "Eye" icon for User Action to keep unread
    • Added "Star" icon to Star/Unstar behavior (and mark in backend)
    • Added MarkRead method that is triggered on expanding element (and mark in backend unless "kept unread")
    • TODO: Added "Share" Icon that needs to be built out more
  • Created Starred component and route for using VirtualScroll to display starredItems fetched from Vuex store
  • Added Typescript Type for FeedItem
  • Added new Vuex Store file for managing items
    • store all items in allItems
    • use getter for starred
    • track total starredCount as separate number that has its own state mutation
    • SET_ITEM mutation replaces item in allItems array
  • Added Unit Tests for new components (except for VirtualScroll)

✅ Checklist

- [ ] Changelog entry added for all important changes. (Can we add the label to prevent the check for changelog? )

📷 Visual

starred component

➡️ Up Next

  • Reuse VirtualScroll component for:
    • display all unread
    • display all items
    • display all items in single feed
    • display all items in a folder
  • Move API Requests to Service Classes (to be shared between stores and prevent stuff like having to duplicate api requests like here…)
  • Sidebar Actions

…r loop

Signed-off-by: Devlin Junker <devlin.junker@gmail.com>
Signed-off-by: Devlin Junker <devlin.junker@gmail.com>
Signed-off-by: Devlin Junker <devlin.junker@gmail.com>
Signed-off-by: Devlin Junker <devlin.junker@gmail.com>
Signed-off-by: Devlin Junker <devlin.junker@gmail.com>
Signed-off-by: Devlin Junker <devlin.junker@gmail.com>
Signed-off-by: Devlin Junker <devlin.junker@gmail.com>
Signed-off-by: Devlin Junker <devlin.junker@gmail.com>
Signed-off-by: Devlin Junker <devlin.junker@gmail.com>
@devlinjunker
Copy link
Contributor Author

😅 phew... this is a huge undertaking! I'll try to get the next PR reusing the VirtualScroll out in the next week or so, hopefully this will start rolling faster after I get a few more of these components under my belt and get used to writing vue unit tests... I think I'm not quite doing it right

@Grotax Grotax merged commit efb1ac2 into nextcloud:vue-rewrite Aug 22, 2023
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants