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

Padding Issue in Navigation Bar #23

Closed
IkelAtomig opened this issue May 13, 2022 · 13 comments
Closed

Padding Issue in Navigation Bar #23

IkelAtomig opened this issue May 13, 2022 · 13 comments
Labels
duplicate This issue or pull request already exists

Comments

@IkelAtomig
Copy link

The Below needs to be fixed. Firefox 100, Windows 11.

image

@IkelAtomig
Copy link
Author

@bmFtZQ Can you tell anything about this ?

@bmFtZQ
Copy link
Owner

bmFtZQ commented May 16, 2022

The permissions button has less padding on the left so the icon can be placed closer to the site information button.

@IkelAtomig
Copy link
Author

But it overlaps for Padlock Icon too. See below :

image

You might need to adjust other permission Icons too as below :

image

@bmFtZQ
Copy link
Owner

bmFtZQ commented May 16, 2022

As I said in issue #4 there isn't really an alternative as CSS selectors can only select downwards in the page layout.

The site information button cannot be selected based on if the permissions button is displayed, so the only solutions I can find are:

  1. Site information button is always shorter on one side.
  2. The button overlaps slightly when the permissions button is displayed.

I found that solution 2 was better than the first, so that is what the theme uses. Although I would welcome anyone to find a better solution and submit a pull request, if possible.

@bmFtZQ bmFtZQ pinned this issue May 16, 2022
@bmFtZQ bmFtZQ added the duplicate This issue or pull request already exists label May 16, 2022
@IkelAtomig
Copy link
Author

IkelAtomig commented May 16, 2022

Why not try using Option 2 and make a commit then ?

By this atleast we don't have permissions icon in the field of Padlock.

@bmFtZQ
Copy link
Owner

bmFtZQ commented May 16, 2022

Why not try using Option 2 and make a commit then ?

By this atleast we don't have permissions icon in the field of Padlock.

Option 2 is what the theme already uses, the button and the icon overlapping is what I meant.

Although I guess there could be a third solution and that would be to add more space between the icons but that didn't look as nice when the icons are not being hovered over imo.

Solution 2 (Current) Solution 3 (more spacing) Solution 3 but with less padding
Solution2 Solution3 Solution3 but with less padding
Solution2 animation Solution3 animation Solution3 but less padding animation

and why I didn't use solution 1:
Screen Shot 2022-05-16 at 22 19 30

@IkelAtomig
Copy link
Author

IkelAtomig commented May 16, 2022

I think Solution 3 (more spacing) is really a good one. You should implement it in my Opinion, I think. It's better to have this instead of all other solutions.

Off-Topic : Would be great if you provide the chrome folder as a package and release it as a version

bmFtZQ pushed a commit that referenced this issue May 16, 2022
@bmFtZQ
Copy link
Owner

bmFtZQ commented May 16, 2022

I have now implemented solution 3 but with solution 2 as optional (uc.tweak.less-permissions-button-padding)

Off-Topic : Would be great if you provide the chrome folder as a package and release it as a version

So releasing tags/versions for each update instead of just downloading the full repository?

@IkelAtomig
Copy link
Author

Great to know.

So releasing tags/versions for each update instead of just downloading the full repository?

Yeah, Because It would be easy to follow new Changes and update accordingly.

Do you know of a easier way to update the CSS instead of downloading again, everytime ?

@bmFtZQ
Copy link
Owner

bmFtZQ commented May 17, 2022

Do you know of a easier way to update the CSS instead of downloading again, everytime ?

You could use a simple script to download the theme and move it to the chrome folder automatically using curl and tar. Although it downloads the latest commit instead of the latest release because that is easier to do.

macOS/Linux Shell script (.sh file):

#!/bin/sh
# temp dir used to store downloaded/extracted files before copying
TEMPCHROMEDIR="/tmp/userchrome"
# find profile dir in about:profiles "root directory"
PROFILEDIR="/Users/${USERNAME}/Library/Application Support/Firefox/Profiles/xxxxxxxx.default-release"
# file to download
UPDATEURL="https://github.com/bmFtZQ/edge-frfox/archive/refs/heads/main.tar.gz"

# make temp dir
mkdir -p "$TEMPCHROMEDIR"
# download file using curl
curl -Lo "$TEMPCHROMEDIR/theme.tar.gz" "$UPDATEURL"
# extract archive into temp dir
tar -xf "$TEMPCHROMEDIR/theme.tar.gz" --strip 1 -C "$TEMPCHROMEDIR"
# remove existing chrome dir
rm -rf "$PROFILEDIR/chrome"
# copy chrome folder to profile
cp -r "${TEMPCHROMEDIR}/chrome" "$PROFILEDIR/chrome"
# remove temp dir
rm -rf "$TEMPCHROMEDIR"

Windows Batch script (.bat file):

@REM temp dir used to store downloaded/extracted files before copying
set TEMPCHROMEDIR=%Temp%\userchrome
@REM find profile dir in about:profiles "root directory"
set PROFILEDIR=C:\Users\%USERNAME%\AppData\Roaming\Mozilla\Firefox\Profiles\xxxxxxxx.default-release
@REM file to download
set UPDATEURL=https://github.com/bmFtZQ/edge-frfox/archive/refs/heads/main.tar.gz

@REM make temp dir
mkdir %TEMPCHROMEDIR%
@REM download file using curl
curl -Lo "%TEMPCHROMEDIR%\theme.tar.gz" "%UPDATEURL%"
@REM extract archive into temp dir
tar -xf "%TEMPCHROMEDIR%\theme.tar.gz" --strip 1 -C "%TEMPCHROMEDIR%"
@REM remove existing chrome dir
rmdir "%PROFILEDIR%\chrome\" /Q/S
@REM copy chrome folder to profile
xcopy "%TEMPCHROMEDIR%\chrome" "%PROFILEDIR%\chrome\" /E/Y
@REM remove temp dir
rmdir "%TEMPCHROMEDIR%" /Q/S

So you can save the script to a file, add the profile directory (can be found in about:profiles) and run it when you want to update the theme. The script is commented and explains what each step does.

@IkelAtomig
Copy link
Author

I think it's better to use Git for this purposes instead.

Okay, What are you going to do about the Releases ?

@IkelAtomig
Copy link
Author

As, The Main Issue is solved and doing releases is dependant on you. I am closing this issue.

@bmFtZQ
Copy link
Owner

bmFtZQ commented Jun 10, 2022

I've published the first new release now.

I think it's better to use Git for this purposes instead.

I didn't use git for the script because that isn't installed by default on Windows and macOS.

@bmFtZQ bmFtZQ unpinned this issue Jun 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

2 participants