mozilla / FirefoxColor

Theming demo for Firefox Quantum and beyond
https://color.firefox.com
Mozilla Public License 2.0
452 stars 93 forks source link

fix: styling of popup #962

Closed himanshujaidka closed 3 years ago

himanshujaidka commented 3 years ago

fix the issue no #883 and update the dependencies or packages Screenshot (122)

himanshujaidka commented 3 years ago

This may work for your specific screen size, but this hardcoded value of 22px is not the universally right fix. For example, shrink the window size (reduce the height of the browser window) and you'll see that there is still some overlap.

And also, please don't put unrelated changes (e.g. dependency updates) together with this patch. It increases the size of the patch and increases the risk of merge conflicts.

okay

himanshujaidka commented 3 years ago

This may work for your specific screen size, but this hardcoded value of 22px is not the universally right fix. For example, shrink the window size (reduce the height of the browser window) and you'll see that there is still some overlap.

And also, please don't put unrelated changes (e.g. dependency updates) together with this patch. It increases the size of the patch and increases the risk of merge conflicts.

It seems to be working fine even in the small shrunk tab or in the mobile view or if this will not work then I have to use then media query

himanshujaidka commented 3 years ago

Made changes

himanshujaidka commented 3 years ago

sir, any updates?

himanshujaidka commented 3 years ago

Please undo the update of the package.json / package-lock.json files. These are unrelated to the patch.

okay, I will do that sorry that I have forgotten to remove it as you have said earlier also

Rob--W commented 3 years ago

Closing in favor of #966, which solves the issue in a better way.

himanshujaidka commented 3 years ago

Closing in favor of #966, which solves the issue in a better way.

Thanks for the guidance :)