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

feat: アイコンデコレーションをアイコンの背後に表示できるように #14708

Draft
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

kakkokari-gtyih
Copy link
Contributor

What

Why

Fix #14707

Additional info (optional)

Checklist

  • Read the contribution guide
  • Test working in a local environment
  • (If needed) Add story of storybook
  • (If needed) Update CHANGELOG.md
  • (If possible) Add tests

CenTdemeern1 and others added 8 commits October 5, 2024 15:16
(cherry picked from commit 28aa99b2737fd3d66f8c438477c96aa94b0059ed)
(cherry picked from commit e43c58a9555b6a241455a11f7671d0e72c37bdfd)
Co-authored-by: dakkar <[email protected]>
(cherry picked from commit cb264dae75d94151e9b2a95aca48dde1e45a9fbb)
(cherry picked from commit 25e90f9c071fc6de07fb50f4adce96f7de1f354f)
@syuilo
Copy link
Member

syuilo commented Oct 5, 2024

below というより behind な気がするわね

@kakkokari-gtyih
Copy link
Contributor Author

below というより behind な気がするわね

たしかに

@github-actions github-actions bot added packages/frontend Client side specific issue/PR packages/backend Server side specific issue/PR packages/misskey-js labels Oct 5, 2024
Copy link

codecov bot commented Oct 5, 2024

Codecov Report

Attention: Patch coverage is 37.83784% with 23 lines in your changes missing coverage. Please review.

Project coverage is 41.29%. Comparing base (043fef9) to head (58856b2).

Files with missing lines Patch % Lines
...nd/src/pages/settings/avatar-decoration.dialog.vue 0.00% 13 Missing ⚠️
...ckages/frontend/src/components/global/MkAvatar.vue 50.00% 3 Missing ⚠️
.../frontend/src/pages/settings/avatar-decoration.vue 0.00% 3 Missing ⚠️
...rc/pages/settings/avatar-decoration.decoration.vue 0.00% 2 Missing ⚠️
...ges/backend/src/core/entities/UserEntityService.ts 0.00% 1 Missing ⚠️
...kages/backend/src/server/api/endpoints/i/update.ts 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #14708      +/-   ##
===========================================
+ Coverage    39.68%   41.29%   +1.61%     
===========================================
  Files         1549     1553       +4     
  Lines       194593   200358    +5765     
  Branches      3603     3624      +21     
===========================================
+ Hits         77223    82744    +5521     
- Misses      116771   117013     +242     
- Partials       599      601       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

github-actions bot commented Oct 5, 2024

このPRによるapi.jsonの差分

差分はこちら
--- base
+++ head
@@ -49834,6 +49834,12 @@
                           ],
                           "maximum": 0.25,
                           "minimum": -0.25
+                        },
+                        "showBehind": {
+                          "type": [
+                            "boolean",
+                            "null"
+                          ]
                         }
                       },
                       "required": [
@@ -76725,6 +76731,9 @@
                 },
                 "offsetY": {
                   "type": "number"
+                },
+                "showBehind": {
+                  "type": "boolean"
                 }
               },
               "required": [

Get diff files from Workflow Page

@kakkokari-gtyih
Copy link
Contributor Author

アイコンの重ね順を自由に設定できるようになった場合、このアプローチだとまずいかもしれない(今は重ね順=配列順になっているけど、このフラグの有無でその順番が崩れてややこしくなる可能性がある)

@kakkokari-gtyih
Copy link
Contributor Author

理想はアイコンも含めた重ね順を指定できるようになることかも(アイコンを0として正負で指定するとか)

@syuilo
Copy link
Member

syuilo commented Oct 5, 2024

このフラグの有無でその順番が崩れるというのがよくわからんわね

@syuilo
Copy link
Member

syuilo commented Oct 5, 2024

配列をフラグの有無でグルーピングした後の並び順で良いんじゃないかしら

@syuilo
Copy link
Member

syuilo commented Oct 5, 2024

実態はひとつの配列だけどフラグがオンのデコレーションの配列とフラグがオフのデコレーションの配列の二つであると解釈する

@kakkokari-gtyih
Copy link
Contributor Author

実態はひとつの配列だけどフラグがオンのデコレーションの配列とフラグがオフのデコレーションの配列の二つであると解釈する

これをするのが面倒そうなので、orderとかのプロパティを生やして実際のアイコン画像を0とした正負の値を設定して明示的に順番を指定したほうがいいんじゃないかと考えた

@syuilo
Copy link
Member

syuilo commented Oct 5, 2024

設定UI上でその情報を表現するのが難しそう

@syuilo
Copy link
Member

syuilo commented Oct 5, 2024

実態はひとつの配列だけどフラグがオンのデコレーションの配列とフラグがオフのデコレーションの配列の二つであると解釈する

これをするのが面倒そう

filterすれば良いだけじゃないかしら

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend Server side specific issue/PR packages/frontend Client side specific issue/PR packages/misskey-js
Projects
None yet
Development

Successfully merging this pull request may close these issues.

アバターデコレーションをアバター画像の後ろに表示できるようにする
3 participants