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

Migrate Popular view to composition API #6129

Conversation

ChunkyProgrammer
Copy link
Member

Migrate Popular to composition API

Pull Request Type

  • Refactoring - Composition API migration

Description

This pull request migrates the Popular view to the composition API.

Testing

  • use invidious as primary api (or enable fallback).
  • go to popular
  • make sure videos load correctly
  • leave page
  • go back
  • make sure videos are loaded from cache (no network connection made to youtube)
  • refresh popular page

Desktop

  • OS: Linux Mint
  • OS Version: 22
  • FreeTube version: 0.22.0

@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Nov 10, 2024
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 10, 2024 05:00
absidue
absidue previously approved these changes Nov 10, 2024
src/renderer/views/Popular/Popular.vue Outdated Show resolved Hide resolved
src/renderer/views/Popular/Popular.vue Outdated Show resolved Hide resolved
@absidue absidue self-requested a review November 10, 2024 20:22
Co-Authored-By: absidue <48293849+absidue@users.noreply.github.com>
Copy link
Member

@absidue absidue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we are no longer trapped by the Options API, if the cache is available we can use it directly for the first render, instead of reading it in onMounted and triggering a re-render.

onMounted(() => {
document.addEventListener('keydown', keyboardShortcutHandler)

shownResults.value = popularCache.value || []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
shownResults.value = popularCache.value || []

const { t } = useI18n()

const isLoading = ref(false)
const shownResults = shallowRef([])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const shownResults = shallowRef([])

src/renderer/views/Popular/Popular.vue Show resolved Hide resolved
document.addEventListener('keydown', keyboardShortcutHandler)

shownResults.value = popularCache.value || []
if (!shownResults.value || shownResults.value.length < 1) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (!shownResults.value || shownResults.value.length < 1) {
if (shownResults.value.length === 0) {

Co-Authored-By: absidue <48293849+absidue@users.noreply.github.com>
Co-authored-by: absidue <48293849+absidue@users.noreply.github.com>
@FreeTubeBot FreeTubeBot merged commit 8ab8b4e into FreeTubeApp:development Nov 15, 2024
5 checks passed
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Nov 15, 2024
Soham456 pushed a commit to Soham456/FreeTube that referenced this pull request Dec 5, 2024
* Migrate Popular to composition API

* remove comment, use shallowRef for shownResults

Co-Authored-By: absidue <48293849+absidue@users.noreply.github.com>

* Optimize setting shownResults

Co-Authored-By: absidue <48293849+absidue@users.noreply.github.com>

* Fix typo

Co-authored-by: absidue <48293849+absidue@users.noreply.github.com>

---------

Co-authored-by: absidue <48293849+absidue@users.noreply.github.com>
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.

5 participants