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

Filter icons for render mode #120

Merged

Conversation

alexey1312
Copy link
Contributor

Added 3 new parameters for filtering icons and denote a asset render mode.

CONFIG.md Outdated
renderModeOriginalSuffix: '_original'
# Configure the suffix for filtering Icons and to denote a asset render mode: "template".
# ❗️ It will work, if renderMode have default value isn't "template". Defaults to nil.
renderModeOriginalSuffix: '_template'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Тут опечатка. Должно быть renderModeTemplateSuffix.

Copy link
Collaborator

@subdan subdan Sep 21, 2021

Choose a reason for hiding this comment

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

Может быть убрать этот параметр? Свойство renderMode обозначает какой режим рендеринга использовать для всех иконок. То есть по умолчанию все иконки будут иметь template режим. Если кто-то хочет у нескольких иконок отключить template режим то он задаст свойство renderModeOriginalSuffix. Кажется что и renderModeDefaultSuffix не нужен.

Copy link
Contributor Author

@alexey1312 alexey1312 Sep 21, 2021

Choose a reason for hiding this comment

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

Тут опечатка. Должно быть renderModeTemplateSuffix.

Опечатку поправил.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Может быть убрать этот параметр? Свойство renderMode обозначает какой режим рендеринга использовать для всех иконок. То есть по умолчанию все иконки будут иметь template режим. Если кто-то хочет у нескольких иконок отключить template режим то он задаст свойство renderModeOriginalSuffix. Кажется что и renderModeDefaultSuffix не нужен.

Если цветные иконки расположены в одном файле, они уже имеют свой суффикс (для темной темы) - их будет невозможно отфильтровать, получается нужно задавать через свойство renderMode для всех и фильтровать через renderModeTemplateSuffix оставшиеся.
Возможны еще какие-то кейсы и кажется лучше больше параметров для большей гибкости, чем их отсутсвие.

Copy link
Collaborator

@subdan subdan Sep 21, 2021

Choose a reason for hiding this comment

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

То есть у иконки темной темы будет двойной суффикс: _dark_template?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Эм, нет.
renderMode = default
Для темной темы будут иконки суффиксом: _dark
ЧБ иконки будут с суффиксом _template и их будем фильтровать через параметр renderModeTemplateSuffix

Copy link
Collaborator

Choose a reason for hiding this comment

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

Но в CONFIG файле написано что renderModeTemplateSuffix будет работать только если renderMode = template.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Вроде нет
❗️ It will work, if renderMode have default value isn't "template". Defaults to nil.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Да, сорри, не заметил отрицание isn't

CONFIG.md Outdated
renderMode: default
# Configure the suffix for filtering Icons and to denote a asset render mode: "default".
# ❗️ It will work, if renderMode have default value is "template". Defaults to nil.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Предлагаю убрать эмоджи. Они отвлекают внимание на себя, хотя эта опция очень редко будет использоваться.

CONFIG.md Outdated
# ❗️ It will work, if renderMode have default value is "template". Defaults to nil.
renderModeDefaultSuffix: '_default'
# Configure the suffix for filtering Icons and to denote a asset render mode: "original".
# ❗️ It will work, if renderMode have default value is "template". Defaults to nil.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# ❗️ It will work, if renderMode have default value is "template". Defaults to nil.
# It will work when renderMode value is "template". Defaults to nil.

Кажется вот так будет правильней с т.з. англ. яз.

CONFIG.md Outdated
# ❗️ It will work, if renderMode have default value is "template". Defaults to nil.
renderModeDefaultSuffix: '_default'
# Configure the suffix for filtering Icons and to denote a asset render mode: "original".
# ❗️ It will work, if renderMode have default value is "template". Defaults to nil.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# ❗️ It will work, if renderMode have default value is "template". Defaults to nil.
# It will work when renderMode value is "template". Defaults to nil.

Кажется вот так будет правильней с т.з. англ. яз.

@subdan subdan merged commit 546212f into RedMadRobot:master Sep 21, 2021
@alexey1312 alexey1312 deleted the feature/filterIconsForRenderMode branch September 21, 2021 22:47
@subdan
Copy link
Collaborator

subdan commented Sep 22, 2021

I've released a new version of figma-export with this feature.

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

Successfully merging this pull request may close these issues.

None yet

2 participants