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

Fix: Update thumbnail url and channel name for subscriptions when change detected #1783

Merged

Conversation

ChunkyProgrammer
Copy link
Member


Update thumbnail url for subscriptions when change detected

Pull Request Type
Please select what type of pull request this is:

  • Bugfix
  • Feature Implementation

Related issue
Closes #1677

Description
When navigating to a channel, video or playlist page: it will check if the user is subscribed to the author & if they are subscribed then it will check if the thumbnail needs updating and will update the thumbnail accordingly

Testing (for code that is not small enough to be easily understandable)
I did the following for Invidious & local api:
I modified the thumbnail url to one of my subscriptions then:

  • navigated to one of their videos (without navigating to channel)
  • navigated to one of their playlists (without navigating to channel)
  • navigated to their channel page.

Desktop (please complete the following information):

  • OS: Windows
  • OS Version: 10
  • FreeTube version: 0.15.0 Beta

Additional context
Add any other context about the problem here.

@PrestonN PrestonN enabled auto-merge (squash) October 2, 2021 21:08
@ChunkyProgrammer ChunkyProgrammer added the PR: waiting for review For PRs that are complete, tested, and ready for review label Oct 2, 2021
@ChunkyProgrammer
Copy link
Member Author

the function to update thumbnails is pretty repetitive so I'm wondering if there's a file I can move it to so it only appears once (I'd update the function to take in thumbnail url & channel Id)

@kommunarr
Copy link
Collaborator

the function to update thumbnails is pretty repetitive so I'm wondering if there's a file I can move it to so it only appears once (I'd update the function to take in thumbnail url & channel Id)

I use this implementation for global functions in the Accessibility Improvements PR:

Vue.prototype.$handleDropdownKeyboardEvent = function(event, afterElement) {

Note that the function name begins with a $ so as to explicitly denote that this function is global.

@ChunkyProgrammer ChunkyProgrammer added PR: WIP and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Oct 3, 2021
@ChunkyProgrammer ChunkyProgrammer added PR: waiting for review For PRs that are complete, tested, and ready for review and removed PR: WIP labels Oct 3, 2021
@Winterhuman
Copy link

Just want to mention something that I've just found since creating #1677, it turns out it's not just channel icons in the subscription list that don't change, the channel name also doesn't update until you unsubscribe and resubscribe to the channel.

@ChunkyProgrammer ChunkyProgrammer added PR: changes requested and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Oct 18, 2021
@ChunkyProgrammer ChunkyProgrammer added PR: waiting for review For PRs that are complete, tested, and ready for review and removed PR: changes requested labels Nov 3, 2021
@ChunkyProgrammer ChunkyProgrammer changed the title Fix: Update thumbnail url for subscriptions when change detected Fix: Update thumbnail url and channel name for subscriptions when change detected Nov 3, 2021
@efb4f5ff-1298-471a-8973-3d47447115dc

@ChunkyProgrammer can u resolve the merge conflicts. i wanted to test but im unable to because of it :(

@PrestonN
Copy link
Member

PrestonN commented Jan 7, 2022

the function to update thumbnails is pretty repetitive so I'm wondering if there's a file I can move it to so it only appears once (I'd update the function to take in thumbnail url & channel Id)

I actually created the Utils Store for this exact reason. The idea is that miscellaneous functions that are used across the app but don't have a good spot to go typically goes into utils.

I'm open to suggestions from you or @jasonhenriquez if this approach should be changed however, but that's how I've been handling it so far.

@ChunkyProgrammer
Copy link
Member Author

Sounds good, I'll move the method to utils when I get the chance

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added PR: changes requested and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Jan 7, 2022
@ChunkyProgrammer ChunkyProgrammer added PR: waiting for review For PRs that are complete, tested, and ready for review and removed PR: changes requested PR: merge conflicts / rebase needed labels Jan 9, 2022
@hockerschwan hockerschwan mentioned this pull request Mar 15, 2022
1 task
Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

Tested locally

  • navigated to one of their videos (without navigating to channel)
  • navigated to one of their playlists (without navigating to channel)
  • navigated to their channel page.

@efb4f5ff-1298-471a-8973-3d47447115dc

Tested locally

  • navigated to one of their videos (without navigating to channel)
  • navigated to one of their playlists (without navigating to channel)
  • navigated to their channel page.

@PikachuEXE No approval😔

src/renderer/views/Playlist/Playlist.js Outdated Show resolved Hide resolved
src/renderer/views/Playlist/Playlist.js Outdated Show resolved Hide resolved
src/renderer/store/modules/profiles.js Outdated Show resolved Hide resolved
src/renderer/store/modules/profiles.js Outdated Show resolved Hide resolved
src/renderer/store/modules/profiles.js Outdated Show resolved Hide resolved
src/renderer/store/modules/profiles.js Outdated Show resolved Hide resolved
src/renderer/store/modules/profiles.js Outdated Show resolved Hide resolved
@PikachuEXE
Copy link
Collaborator

@efb4f5ff-1298-471a-8973-3d47447115dc
Feature review & code review are separate :)

@efb4f5ff-1298-471a-8973-3d47447115dc

@efb4f5ff-1298-471a-8973-3d47447115dc
Feature review & code review are separate :)

Yeah that's my bad for not noticing that 😬

src/renderer/views/Playlist/Playlist.js Outdated Show resolved Hide resolved
src/renderer/views/Watch/Watch.js Outdated Show resolved Hide resolved
src/renderer/views/Watch/Watch.js Outdated Show resolved Hide resolved
@absidue absidue added PR: changes requested and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Jun 2, 2022
@ChunkyProgrammer ChunkyProgrammer added PR: waiting for review For PRs that are complete, tested, and ready for review and removed PR: changes requested labels Jun 3, 2022
Copy link
Member

Choose a reason for hiding this comment

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

LGTM!

@PrestonN PrestonN merged commit 74dc309 into FreeTubeApp:development Jun 3, 2022
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Jun 3, 2022
@ChunkyProgrammer ChunkyProgrammer deleted the update-channel-thumb branch June 27, 2023 14:14
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.

Channel icon in subscription list not updating with change
8 participants