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

Use update flutter app icon (#510) #537

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

maksnevor
Copy link

@maksnevor maksnevor commented Aug 31, 2023

Resolves #510

Synopsis

Неудобно обновлять иконки, нужно не забыть обновить их для каждой платформы в директориях android, ios, windows, macos, linux, web.

Solution

Использовать пакет update-flutter-app-icon, к нему написать update_launcher.json, который будет обновлять иконки во всем приложении.

Checklist

  • Created PR:
    • In draft mode
    • Name contains issue reference
    • Has type and k:: labels applied
  • Before review:
    • Documentation is updated (if required)
    • Tests are updated (if required)
    • Changes conform code style
    • CHANGELOG entry is added (if required)
    • FCM (final commit message) is posted or updated
    • Draft mode is removed
  • Review is completed and changes are approved
    • FCM (final commit message) is approved
  • Before merge:
    • Milestone is set
    • PR's name and description are correct and up-to-date
    • All temporary labels are removed

@maksnevor maksnevor added enhancement Improvement of existing features or bugfix k::toolchain Changes of project toolchain labels Aug 31, 2023
@maksnevor maksnevor added this to the 0.1.0-alpha.10 milestone Aug 31, 2023
@maksnevor maksnevor self-assigned this Aug 31, 2023
@maksnevor
Copy link
Author

FCM

Update launcher icons with update-flutter-app-icon (#537, #510)

@maksnevor maksnevor marked this pull request as ready for review September 4, 2023 05:38
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
update_icons.json Outdated Show resolved Hide resolved
snap/gui/app_icon.desktop Outdated Show resolved Hide resolved
assets/icons/launcher/android_launcher.png Outdated Show resolved Hide resolved
assets/icons/launcher/ios_launcher.png Outdated Show resolved Hide resolved
Copy link
Contributor

@SleepySquash SleepySquash left a comment

Choose a reason for hiding this comment

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

@maksnevor, отписал кое-где просьбы поправить, кое-где сомнения.

Copy link
Contributor

Choose a reason for hiding this comment

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

@maksnevor, а почему appleа два? apple.png и apple_rounded? Если один для iOS, а второй для macOS, то какой смысл тогда их называть apple?

Copy link
Author

Choose a reason for hiding this comment

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

apple - для ios,android. apple_rounded - используется для macos форматов @2x@2x.

Copy link
Contributor

Choose a reason for hiding this comment

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

@maksnevor, вообще смотрю и думаю об этом всём: чтобы сделать хорошую иконку, нужно так или иначе открывать фотошоп и нарезать эту иконку. Существуют всякие веб сервисы, которые генерируют иконки (они сразу генерируют пачку). Какой алгоритм действий должен быть, чтобы обновить иконки? Открыть фотошоп и нарезать 8-10 картинок?

Copy link
Author

Choose a reason for hiding this comment

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

Существуют, но генерируют они плохие пачки. Теряем много адаптации и форматов.

Copy link
Contributor

Choose a reason for hiding this comment

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

@maksnevor, https://icon.kitchen/ генерит хорошие пачки.

icon.update:
ifeq ($(dockerized),yes)
docker run --rm -v "$(PWD)":/app -w /app \
-v "$(HOME)/.pub-cache":/usr/local/flutter/.pub-cache \
Copy link
Contributor

Choose a reason for hiding this comment

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

А зачем у этого конейтера маппить .pub-cache? Это же нода, она не использует .pub-cache.

ifeq ($(dockerized),yes)
docker run --rm -v "$(PWD)":/app -w /app \
-v "$(HOME)/.pub-cache":/usr/local/flutter/.pub-cache \
node:${NODE_VER} \
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. А NODE_VER - это что? Оно где-то определено?
  2. В Makefile'е везде сейчас используется формат записи переменных среды в виде $( ) - с круглыми скобками. Давайте будем консистентны и придержимся единого формата.

Copy link
Contributor

Choose a reason for hiding this comment

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

А почему иконка 256х256 квардаратная, а, например, 16@[email protected] - скруглённая с отступами?

  1. Это же разные размеры одной и той же иконки?
  2. А почему @2x@2x?

make icon.update dockerized=yes # Docker-wrapped
```

__Note:__ To update, you need to download new icons along the path `assets/icons/launcher` and match the correct naming:
Copy link
Contributor

Choose a reason for hiding this comment

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

Иконки же не скачиваются. Их скорее размещают, т.к. где-то они уже лежат. И помещают скорее изображения, из которых будут нерезаны иконки.

* `linux.png` - for linux launcher icon
* `web_alert.png` - for web launcher icon (alert)
* `web.png` - for web launcher icon and favicon
* `windows.png` - for windows launcher icon
Copy link
Contributor

Choose a reason for hiding this comment

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

Я не понял, почему иконка 256 на 256 сначала, а потом, прочитав инструкцию от Microsoft, понял, что на винде иконка 256 - это максимально оптимальных размер для иконки.

У меня в голове пока 2 сценария:

  1. Если делаем для каждой ОС свои, то нужно описать требования к изображениям, которые туда кладутся.
  2. Делаем одну 1024 на 1024 или SVG картиночку (возможно, две - бэкграунд и форграунд, т.к. андойд так требует), из которой как-то самостоятельно нарезаются иконки под все ОС, потому что все эти требования прочитать и учесть - тоже не очень приятная работа.

Makefile Outdated
node:${NODE_VER} \
make icon.update dockerized=no
else
npx update-flutter-app-icon dev/update_icons.json
Copy link
Contributor

Choose a reason for hiding this comment

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

При выполнении пишет такое:

mayday@MBP messenger % make icon.update
npx update-flutter-app-icon dev/update_icons.json
Need to install the following packages:
  [email protected]
Ok to proceed? (y) 

Т.е. это надо устанавливать? А куда? Не лучше тогда в npm i --dev update-flutter-app-icon какой-нибудь делать, чтобы через npm вызывать?

Copy link
Contributor

Choose a reason for hiding this comment

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

Не app_icon, а icon должен быть префикс у файлов. .appiconset же имеет файл Contents.json, там написаны префиксы всех иконок. Аналогично и с iOS.

Makefile Outdated
node:${NODE_VER} \
make icon.update dockerized=no
else
npx update-flutter-app-icon dev/update_icons.json
Copy link
Contributor

Choose a reason for hiding this comment

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

При выполнении мне пишет вот такое что-то (иконка там есть, но не обновляется):

=== windows ===
/Users/mayday/Developer/Flutter/messenger/windows/runner/resources/app_icon.ico [{"size":-1}]
Error converting file "/Users/mayday/Developer/Flutter/messenger/windows/runner/resources/app_icon.ico": RangeError [ERR_OUT_OF_RANGE]: The value of "value" is out of range. It must be >= 0 and <= 255. Received 319

@SleepySquash SleepySquash marked this pull request as draft September 5, 2023 15:58
@SleepySquash SleepySquash added the p::minor Issues/PRs which are not required to be resolved at the moment (temporary) label Nov 27, 2023
@SleepySquash SleepySquash removed this from the 0.1.0-alpha.12 milestone Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement of existing features or bugfix k::toolchain Changes of project toolchain p::minor Issues/PRs which are not required to be resolved at the moment (temporary)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use flutter_launcher_icons to generate icons
2 participants