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

Quick fix #1597

Merged
merged 4 commits into from
Mar 4, 2023
Merged

Quick fix #1597

merged 4 commits into from
Mar 4, 2023

Conversation

dodieboy
Copy link
Member

@dodieboy dodieboy commented Mar 4, 2023

No description provided.

dodieboy added 2 commits March 4, 2023 10:43
Rewrite the whole theme cookie setting code, now it will use ImprovedTube.setCookie()
@dodieboy
Copy link
Member Author

dodieboy commented Mar 4, 2023

Glad to hear from you! @dodieboy Are you on Discord?

work

may we help you anything too?🤫😊

BTW / for later this fix was not moved to v4? https://github.com/code-for-charity/ImprovedTube-for-YouTube/pull/1372/files #1393 also persists (with unlimied page width) ( #1407 )

@ImprovedTube I added more check for my cookie code to prevent error and also port over my player element overlap code to v4 alr

@ImprovedTube
Copy link
Member

thanks & happy weekend @dodieboy

(

  • so you decided against location.reload(); for darkmode?
  • people werent able to undo darkmode before unless deleting cookies. So, without looking at it, i wondered to remove the option and leave it to YouTube's menu

)

@dodieboy
Copy link
Member Author

dodieboy commented Mar 4, 2023

location.reload(); may cause a reload loop, so if need, we will suggest user to reload by themself better than the extension keep reloading that cause memory problem to user.

I tested that youtube now set and delete the cookie when user set the appearance in the setting, so user will mot need to know how to delete those cookie when the not using our extension, they can just go appearance and reset those setting.

@dodieboy
Copy link
Member Author

dodieboy commented Mar 4, 2023

btw i got 1 more fix, so dont merge this PR yet

@dodieboy
Copy link
Member Author

dodieboy commented Mar 4, 2023

@ImprovedTube I'm done with fixing thing, can merge this PR after review. Thanks 😄

@ImprovedTube
Copy link
Member

ImprovedTube commented Mar 4, 2023

@ImprovedTube ImprovedTube merged commit 67a6478 into code-charity:master Mar 4, 2023
@dodieboy
Copy link
Member Author

dodieboy commented Mar 4, 2023

On my side, I got an error which show it is not able to find the parent parent node,therefore, i change the check to use the new class name which youtube just added.

I change the SVG method too due to the new change. The button is taking too long to load, resulting in queryselector not able to find the element needed. So I change to this method for now. However I wish that I can bring back the SVG method as we will have lesser work when they added more button there and the extension will hide the correct button even when youtube have a new update

@dodieboy dodieboy deleted the Fix branch March 5, 2023 02:22
@ImprovedTube
Copy link
Member

  • darkmode: sorry this brings back a familiar issue, that only the header is black (@dodieboy)
    (=setting darktheme in youtube & no settings in Improvedtube)

    • also: light theme in youtube wont take effect if setting dark in the extension).
      • maybe we could set both the same way the youtube does, so that it would change in Youtube's menu too)
  • detail buttons: (only) the first 2-3 are matched by CSS yet

hope you get enough rest too😳

@ImprovedTube ImprovedTube mentioned this pull request Mar 13, 2023
7 tasks
ImprovedTube added a commit that referenced this pull request Apr 4, 2023
ImprovedTube added a commit that referenced this pull request Jan 12, 2024
ImprovedTube added a commit that referenced this pull request Jan 12, 2024
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.

2 participants