monero-project / monero-site

https://getmonero.org
BSD 3-Clause "New" or "Revised" License
275 stars 384 forks source link

css/custom.css: correct hangouts icon coordinates #2314

Closed plowsof closed 2 months ago

plowsof commented 2 months ago

replaces / includes #2307 *Add discord to hangouts

opening the spritesheet in GIMP to obtain pixel correct values:

Icon Default XY Hover XY Size pxl spacing below between
Facebook 128 0 176 0 43x43 5 5
Twitter 128 48 176 48 43x43 5 5
Reddit 128 96 176 96 43x43 5 5
Github 128 144 176 144 43x43 5 5
Gitlab 128 190 176 190 43x43 3 5
Telegram 127 236 176 236 45x45 x 4

all issues fixed in this PR:

so we now have 43+5 actual spacing to avoid confusing the next generation

netlify[bot] commented 2 months ago

Deploy Preview for barolo-time-757cf9 ready!

Built without sensitive environment variables

Name Link
Latest commit dcdee73998bb02b1d0f9c8e1c908a1abb62841e0
Latest deploy log https://app.netlify.com/sites/barolo-time-757cf9/deploys/66928888e2706800084be435
Deploy Preview https://deploy-preview-2314--barolo-time-757cf9.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

plowsof commented 2 months ago

appears to be fine: https://discord.gg/SyGUMWBqvF (just not working for me as ive not verified my discord account)

nahuhh commented 2 months ago

https://discord.com/invite/SyGUMWBqvF

does this one work?

HardenedSteel commented 2 months ago

can we just split the icons? which would be easier to work in future

nahuhh commented 2 months ago

can we just split the icons? which would be easier to work in future

We were already using a (broken) sprite sheet. This PR has 2 commits.

  1. fixes the broken sheet
  2. pulls in discord commit

IMO, if we want to change the sprite to individual icons, that is a new PR

plowsof commented 2 months ago

sprites have pros/cons. reducing network requests to getmonero by allowing clients to used the cached image seems beneficial enough to just fix/set and forget as we don't work with them often/ever. a PR adding new separate icons would only be warranted if it came with a redesign or quality increase without any increase in network load to the server using fancy new tech (im not qualified enough to produce such a PR of course)

*theres a plugin for that^ works great.

plowsof commented 2 months ago

ignore my above comment about lazy-loading, since 2020 its a default in most/all browsers. the fact that we have a sprite image on our website is embarrassing. we have separate icon files for other things so this is just a relic of the past and needs to be deprecated / replaced with separate, updated icons (which involves design work + style edits to display correctly), but, as @nahuhh mentions above, this PR is fixing the current broken icons/coordinates and pulling in a new discord icon + reducing the original sprite image size by 29% after the work was already done and reviewed so i support this merge. from this point on, no more edits to sprites!

nahuhh commented 2 months ago

also:

- Maintainers SHALL merge correct patches from other Contributors rapidly.
- Any Contributor who has value judgments on a correct patch SHOULD express these via their own patches.