-
Notifications
You must be signed in to change notification settings - Fork 18
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
Add compatibility with DeArrow #34
Conversation
@@ -0,0 +1,18 @@ | |||
.ta-button { | |||
display: flex; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevent DeArrow's hidden titles from being overriden by moving it to a css style from a class.
display: 'flex', | ||
gap: '15px', | ||
}); | ||
titleContainer.classList.add("ta-title-container"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class is also used on DeArrow's end to edit some styles
if (titleContainer.style.display === "-webkit-box") { | ||
// Compatibility with DeArrow | ||
titleContainer.style.setProperty("display", "flex", "important"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depending on which one gets to modifying it first, one of them will be done first. I have put this compatibility one on both sides. Only visible titles will get -webkit-box
, once a title needs to be made invisible, it will correctly override TubeArchivist's modification
if (titleContainer.style.display === "-webkit-box") { | ||
// Compatibility with DeArrow | ||
titleContainer.style.setProperty("display", "flex", "important"); | ||
} | ||
|
||
titleContainer.classList.add('title-container'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this class is used, but didn't want to remove it in case I was missing something. Figured I'd use a more specific class
Thanks for implementing this so fast. I'll take a look at this as soon as I can. |
Thank you for taking a look at this. Unfortunately the buttons disappear if you don't have the dearrow extension. I couldn't quite track it down why this is happening though... |
Oh shoot, should have tested that sorry |
Was there any more progress on this? I've hit the same issue and I'm willing to pick up the PR if needed. |
Having the same issue with DeArrow. Hopefully this can get fixed, had to remove companion extension because of this. |
The progress is what you see. If you want to take a stab at this, please do. |
I took a look at the code, but I'm somewhat struggling with the changes in #30. The extension is broken for me on the current master since that PR was merged. |
closing this in favor of #37. |
Fixes #33
Requires ajayyy/DeArrow@9bed98e
If you want to test with DeArrow you need a beta: https://github.com/ajayyy/DeArrow/actions/runs/7574492134
To test you can use this code: F3ZYy-887ed