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: Observe notification read and fix #6406 #6407

Merged
merged 11 commits into from
Jun 4, 2020

Conversation

tamaina
Copy link
Member

@tamaina tamaina commented May 24, 2020

@tamaina
Copy link
Member Author

tamaina commented May 26, 2020

通知ウィジェットを表示していたり通知ページにいた場合、既読をつける処理をしないとだ

@tamaina
Copy link
Member Author

tamaina commented May 26, 2020

notificationのPack時にisReadがなくて、各通知の既読状態を知る手立てがないんだよね。含めるようにしちゃっていいかしら?

@tamaina tamaina changed the title #6406 のつづき feat: Observe notification read and fix #6406 May 26, 2020
@tamaina tamaina requested review from mei23, syuilo and acid-chicken and removed request for mei23 and syuilo May 26, 2020 05:43
@tamaina
Copy link
Member Author

tamaina commented May 26, 2020

post-form.vueが #6408 と競合した

@tamaina
Copy link
Member Author

tamaina commented May 26, 2020

通知表示検出もc2.a9z.devで動作中

@tamaina tamaina requested a review from syuilo May 26, 2020 07:42
@syuilo
Copy link
Member

syuilo commented May 31, 2020

isRead: falseな通知がAPIから返ってくるケースってあるかな?
普通通知取得APIをたたくとすべて既読になるから常にisReadはtrueになる

@tamaina
Copy link
Member Author

tamaina commented May 31, 2020

ストリームからきた通知はfalseなのでそこらへんで

@tamaina
Copy link
Member Author

tamaina commented May 31, 2020

isReadが最初からfalseなときはIntersectionObserverのインスタンスは無駄ですね

@tamaina
Copy link
Member Author

tamaina commented May 31, 2020

これ、メンションなどのnoteの場合は既読にならないですね

tamaina and others added 2 commits June 3, 2020 13:42
Co-authored-by: syuilo <Syuilotan@yahoo.co.jp>
Co-authored-by: syuilo <Syuilotan@yahoo.co.jp>
@@ -0,0 +1,3 @@
export const notificationTypes = ['follow', 'mention', 'reply', 'renote', 'quote', 'reaction', 'pollVote', 'receiveFollowRequest', 'followRequestAccepted', 'groupInvited', 'app'] as const;
Copy link
Member

@syuilo syuilo Jun 4, 2020

Choose a reason for hiding this comment

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

notificationTypes は src/models/entities/notification にあっても良さそうに思った

@@ -0,0 +1,3 @@
export const notificationTypes = ['follow', 'mention', 'reply', 'renote', 'quote', 'reaction', 'pollVote', 'receiveFollowRequest', 'followRequestAccepted', 'groupInvited', 'app'] as const;

export const noteVisibilities = ['public', 'home', 'followers', 'specified'] as const;
Copy link
Member

@syuilo syuilo Jun 4, 2020

Choose a reason for hiding this comment

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

noteVisibilities は src/models/entities/note にあっても良さそう

@tamaina
Copy link
Member Author

tamaina commented Jun 4, 2020

クライアントで使用する場合にmodels/entitiesにあるtsから持ってきても大丈夫なのでしょうか?

怖いけど適当な場所がなかったのでtypes.tsを作ったのですが

@syuilo
Copy link
Member

syuilo commented Jun 4, 2020

うーむ
一旦考える

@syuilo syuilo merged commit 66de51c into misskey-dev:develop Jun 4, 2020
@syuilo
Copy link
Member

syuilo commented Jun 4, 2020

🙏

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