-
Notifications
You must be signed in to change notification settings - Fork 24
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
Regression on showing icons in MacOS full screen mode #51
Comments
Ok did some commit digging. Upto this commit (HEAD~2) all works fine.
Yep its the commit tagged v2024.05.15 that introduced the bug. I git reset HEAD~2 and then cherry-pick 1cb9d37 and all is working. So skipping commit ae64731 (tag: v2024.05.15) fixed it for me but now what else did this introduce! There are so many files changed in this commit that it will take me time to narrow it down to exactly what removal or addition of code did it. For now I'm tired, its late and I'm calling it as I've got it working they way it was previously. Will see if I can make some time to dig some more. |
Nope its more than then extensions/sidebar.css file. Checking out that file 1 back does bring the behaviour back but the formatting of the side bar is then messed up.. sigh. Its going to take some serious rework on v2024.05.15 to unwind this. I think it would be better if @drannex42 took a look as the author of this commit. Love the work done, and much appreciated! |
Ok I couldn't let it go, I moved to head and then reset the extensions/sidebar.css file to one commit back (the one before v2024.05.15) and it works as it should now. So the issue is within all the lines of clean up that were done. At least I was able to confirm the commit and the file to which the changes were made. |
Ok I fixed it. Sidebar.css:39 #main-window[sizemode="fullscreen"] #sidebar-box{ --uc-sidebar-width: 1px; } This was added as a new feature to specifically hide the sidebar (or more specifically reduce it to 1px). Commenting this out fixes the regression but as I suspected this looks like to be intended behaviour. My question to @drannex42, can this be a configurable option? For now I can just comment out this line in my personal copy but as I do not "own" this repo, I didn't want to push up a pull-request without asking and discussing if there is an option to make this configurable. |
Absolutely. I'll try to find a way to make it work. The need to reduce to 1px is to allow the sidebar to be used in fullscreen mode, but still "hide" the sidebat, but wanting to see the icons while in full screen mode makes sense to be an enabled option! I'll add it as an option, should be an easy one! Might also give me an excuse to move a good bit of those options out of the CSS and into about:config. I'll keep this issue open until a new release is posted. Thanks for the detective work on this! |
Np. I was wondering, do you think its worth investigating how to make a FF addon with all this work? It would reach more users and really bring a community of users to leveraging this code. I understand there might still be some manual steps like importing the settings into Sidebery, but barring that... maybe something to think about? The FF addon would allow you to have settings to enable for it much like we were discussing. I could look to help also if this is a direction we would like to explore. |
On MacOS 14.5 Sonoma, after updating my chrome folder to the latest revisions to fix an annoying but that I had with the address bar being covered by the search bar, I no longer see the icons to the left like in Edge in full screen mode!
Now the thing is, this used to work great and in my previous version I could use FF in full screen mode with the vertical scroll icons down the left. Now I'm not sure if this is a bug as it is behaving 100% like Edge does but it was a feature I really enjoyed using.
Could you help me understand what I need to add/remove so I can get this behaviour back?
FF Version 127.0
chrome folder at latest: commit 1cb9d37 (HEAD -> main, origin/main, origin/HEAD)
(1 commit past v2024.05.15
The text was updated successfully, but these errors were encountered: