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

fix: Properly calculate intermediate icon for touch icon generation #47670

Merged
merged 1 commit into from
Sep 2, 2024

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Sep 1, 2024

Fix touch icon rendering that is used by iOS devices if you add a webpage to the home screen. While the previous logic used to work it seems these days imagick does not have a resolution set for svg files, so we need to change calculation slightly so that our intermediate image has the right sizing for the final icon.

(The resolution naming seems confusing here, it is rather the density https://www.php.net/manual/en/imagick.setimageresolution.php#96182)

Before After
image image

@juliusknorr juliusknorr added bug 3. to review Waiting for reviews labels Sep 1, 2024
@juliusknorr juliusknorr requested review from susnux, skjnldsv, a team, ArtificialOwl and Altahrim and removed request for a team September 1, 2024 09:50
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code makes sense

@AndyScherzinger AndyScherzinger added this to the Nextcloud 31 milestone Sep 2, 2024
@juliusknorr juliusknorr merged commit 7964058 into master Sep 2, 2024
172 of 174 checks passed
@juliusknorr juliusknorr deleted the fix/touchicon-calc branch September 2, 2024 11:41
@juliusknorr
Copy link
Member Author

/backport to stable30

Copy link

backportbot bot commented Sep 2, 2024

The backport to stable30 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable30
git pull origin stable30

# Create the new backport branch
git checkout -b backport/47670/stable30

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 463c21d2

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/47670/stable30

Error: Failed to clone repository: Failed to checkout branches: Updating 1cc7851..7964058
From https://github.com/nextcloud/server

  • [deleted] (none) -> origin/7935-download-files-via-post
  • [deleted] (none) -> origin/JonathanTreffler-stop-spamming-deprecations
  • [deleted] (none) -> origin/add-clear-add-to-user-status-public-api
  • [deleted] (none) -> origin/add-integration-tests-for-getting-folder-sizes
  • [deleted] (none) -> origin/add-vtimezone-data-when-creating-personal-calendar
  • [deleted] (none) -> origin/admin_audit/enh/move-to-event-listeners
  • [deleted] (none) -> origin/artonge/feat/download_providers
  • [deleted] (none) -> origin/bug/files-scroll
  • [deleted] (none) -> origin/bugfix/38171/revert-status-when-overwritten
  • [deleted] (none) -> origin/bugfix/avoid-extra-stream-copy
  • [deleted] (none) -> origin/bugfix/noid/unavailable-shares
  • [deleted] (none) -> origin/cache-mimtype-mapping
  • [deleted] (none) -> origin/chore/catch-missing-non-optional-controller-parameter
  • [deleted] (none) -> origin/chore/security/log-password-confirmation-user-backend
  • [deleted] (none) -> origin/dependabot/npm_and_yarn/core-js-3.38.1
  • [deleted] (none) -> origin/direct-access-shared-calendar
  • [deleted] (none) -> origin/email-template-html-fragment
  • [deleted] (none) -> origin/enh/a11y-util
  • [deleted] (none) -> origin/enh/noid/iconfig
  • [deleted] (none) -> origin/enh/noid/use-taskprocessing-in-old-managers
  • [deleted] (none) -> origin/enhancement/caldav-resources-sync-command
  • [deleted] (none) -> origin/enhancement/typed-db-entity
  • [deleted] (none) -> origin/external-list-for
  • [deleted] (none) -> origin/extract-caldav-sharing-plugin
  • [deleted] (none) -> origin/feat-add-iavaialble-in-maintenance-mode
  • [deleted] (none) -> origin/feat/mail-admin-vue
  • [deleted] (none) -> origin/file-cache-insertion-atomic
  • [deleted] (none) -> origin/fix/44160/ctrl-f-trigger-global-search
  • [deleted] (none) -> origin/fix/files-non-visual-loading-info
  • [deleted] (none) -> origin/fix/header-styles
  • [deleted] (none) -> origin/fix/noid/identity-proof-key-checksum
  • [deleted] (none) -> origin/fix/user-settings-admin
  • [deleted] (none) -> origin/home-storage-lazy-datadir
  • [deleted] (none) -> origin/introduce/orm
  • [deleted] (none) -> origin/locate-key-fix
  • [deleted] (none) -> origin/mountcache-lazy-user
  • [deleted] (none) -> origin/multi-object-store
  • [deleted] (none) -> origin/remove_depreated_files
  • [deleted] (none) -> origin/setupmanager-lazy-user
  • [deleted] (none) -> origin/share-null-source
  • [deleted] (none) -> origin/storage-cache-init-in-transaction
  • [deleted] (none) -> origin/trashbin-skip-logging
  • [deleted] (none) -> origin/upload-chunk-locking
  • [deleted] (none) -> origin/use_HSTS
  • [deleted] (none) -> origin/user-files-debug-info
  • [deleted] (none) -> origin/work/sharing_trash
  • 5c9d32b...1281f44 fix/appstore-upgrade-failure -> origin/fix/appstore-upgrade-failure (forced update)
    error: Your local changes to the following files would be overwritten by merge:
    dist/4254-4254.js.map.license
    dist/4696-4696.js.map.license
    dist/5643-5643.js.map.license
    Please commit your changes or stash them before you merge.
    Aborting

Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@juliusknorr
Copy link
Member Author

/backport to stable30

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants