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(frontend): デッキUI・PageWindowでmatchAllに入ったら新しいタブで開くように #13768

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

kakkokari-gtyih
Copy link
Contributor

@kakkokari-gtyih kakkokari-gtyih commented Apr 29, 2024

What

Why

Fix #13766

Additional info (optional)

navHook内でresolveするようになったので、2度resolveが回らないようにnavHookからresolve結果を返すこともできるようにした

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

@github-actions github-actions bot added the packages/frontend Client side specific issue/PR label Apr 29, 2024
Copy link

codecov bot commented Apr 29, 2024

Codecov Report

Attention: Patch coverage is 55.17241% with 13 lines in your changes are missing coverage. Please review.

Project coverage is 77.82%. Comparing base (4579be0) to head (50750d9).
Report is 1 commits behind head on develop.

Files Patch % Lines
packages/frontend/src/nirax.ts 35.00% 13 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           develop   #13768       +/-   ##
============================================
+ Coverage    66.36%   77.82%   +11.45%     
============================================
  Files          990      185      -805     
  Lines       116839    25614    -91225     
  Branches      5792      487     -5305     
============================================
- Hits         77546    19935    -57611     
+ Misses       39262     5672    -33590     
+ Partials        31        7       -24     

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

@anatawa12
Copy link
Member

deck uiからもそうですがウィンドウも新規タブであってほしい

@kakkokari-gtyih
Copy link
Contributor Author

kakkokari-gtyih commented Apr 29, 2024

内部リンク(Misskey本体の機能であるため)には変わりないので基本的に内部で開くのでいいとおもうけど
(Deckは設計上それがむずいので外部タブにしてるだけ)

@kakkokari-gtyih
Copy link
Contributor Author

kakkokari-gtyih commented Apr 29, 2024

内部リンク(Misskey本体の機能であるため)には変わりないので基本的に内部で開くのでいいとおもうけど (Deckは設計上それがむずいので外部タブにしてるだけ)

あと新規タブで開くならリンクにこのアイコン(↓)をつける必要があり、そうなるとリンクコンポーネントが読み込まれた時点ですべてresolveする or 特殊パスを決め打ちしておく必要がありそれはそれでどうなのという気持ちもある

image

@anatawa12
Copy link
Member

image
うーんこの状態で裏を無視してページ全体が置き換えられるのは直感に反すると思うんですよね

@kakkokari-gtyih
Copy link
Contributor Author

(Deckは設計上それがむずいので外部タブにしてるだけ)

なのでdeckは新規タブにしてる(MkYouTubePlayer経由でiframeとかもできるけどそこまでするアレはないと判断)

@anatawa12
Copy link
Member

うーんこの状態で裏を無視してページ全体が置き換えられるのは直感に反すると思うんですよね

のパターンもこれで対応でしたかすみません(deckのtlから直接のみと勘違いしてました)

@kakkokari-gtyih kakkokari-gtyih changed the title fix(frontend): デッキUIでmatchAllに入ったら新しいタブで開くように fix(frontend): デッキUI・PageWindowでmatchAllに入ったら新しいタブで開くように Apr 30, 2024
@kakkokari-gtyih
Copy link
Contributor Author

👀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/frontend Client side specific issue/PR
Projects
None yet
2 participants