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.

Sign up for GitHub

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

Jump to bottom

feature(client): Timeline page for non-login users #8927

Closed
wants to merge 7 commits into from
Closed

feature(client): Timeline page for non-login users #8927

wants to merge 7 commits into from

Conversation

futchitwo
Copy link
Contributor

What

非ログイン時でもタイムラインが使えるように

Why

Additional info (optional)

router.ts を見たところindexページにはglobalCacheKeyがついていたがこちらにも付けた方が良かった方かは不明

@github-actions github-actions bot added the packages/frontend Client side specific issue/PR label Jul 2, 2022
@robflop robflop mentioned this pull request Jul 2, 2022
Closed
syuilo
syuilo reviewed Jul 2, 2022
View reviewed changes
packages/client/src/components/global/page-header.vue Outdated
@@ -71,6 +73,7 @@ const hideTitle = inject('shouldOmitHeaderTitle', false);
const thin_ = props.thin || inject('shouldHeaderThin', false);

const el = $ref<HTMLElement | null>(null);
const tabs = $i ? props.tabs : props.tabs?.filter(tab => !tab.loginRequired);
Copy link
Member

Choose a reason for hiding this comment

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

どんなタブを表示するかという制御は親側で行った方が分かりやすそう
(loginRequired は消して、timeline.vue 側でタブを出しわける)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

スプレッド代入だらけになるか、非ログイン時用とログイン時用で2回定義することになりそう
いっそヘッダーの方弄った方が他のページでも使えるし良いかなと思ったんだけど

Copy link
Member

Choose a reason for hiding this comment

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

スプレッド代入だらけになるか、非ログイン時用とログイン時用で2回定義することになりそう

コード量は増えるけどそっちの方が設計的にはきれいな気がする

いっそヘッダーの方弄った方が他のページでも使えるし良いかなと思ったんだけど

ふーむ
悩むわね

@ThatOneCalculator
Copy link
Contributor

Build fails.

✓ 731 modules transformed.
[vite:esbuild] Transform failed with 1 error:
/home/misskey/misskey-new/packages/client/src/ui/visitor/header.vue?vue&type=script&lang.ts:20:23: ERROR: Unexpected "="
file: /home/misskey/misskey-new/packages/client/src/ui/visitor/header.vue?vue&type=script&lang.ts:20:23

Unexpected "="
18 |  			narrow: null,
19 |  			showMenu: false,
20 |  			isTimelineAvailable = !instance.disableLocalTimeline || !instance.disableGlobalTimeline,
   |                         ^
21 |  		};
22 |  	},

@futchitwo
Copy link
Contributor Author

headerTabとかcomputed使う必要ない気もする

@futchitwo
Copy link
Contributor Author

コンフリクト解決

@tamaina tamaina requested a review from syuilo November 4, 2022 13:54
@tamaina
Copy link
Member

tamaina commented Jan 16, 2023

またコンフリクトしていますが解消されますか？

@futchitwo
Copy link
Contributor Author

コンフリクト解消するか…

#9747 を地味にここで修正してて、代わりに(勝手に)ギャラリーとページのリンクを追加してたんだけど
(playを入れると流石にレイアウトによっては収まらなくなるな)

ロール機能でタイムライン開閉あたりが変わったり、アイコン変わったりしててコンフリ解消するよりこっちは閉じて新しくPR作った方が楽説がある たいした変更量じゃ無いので…

@futchitwo futchitwo mentioned this pull request Feb 4, 2023
Merged
@futchitwo
Copy link
Contributor Author

#9795 でやり直した

@futchitwo futchitwo closed this Feb 4, 2023
@futchitwo futchitwo deleted the logout-timeline branch February 9, 2023 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@syuilo syuilo Awaiting requested review from syuilo

Assignees
No one assigned
Labels
packages/frontend Client side specific issue/PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

非ログイン状態でもタイムラインが使えるように
4 participants
@futchitwo @ThatOneCalculator @tamaina @syuilo