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

UI プロトタイプ実装 #13

Merged
merged 91 commits into from
Feb 12, 2025
Merged

UI プロトタイプ実装 #13

merged 91 commits into from
Feb 12, 2025

Conversation

sushichan044
Copy link
Collaborator

@sushichan044 sushichan044 commented Feb 12, 2025

やったこと

  • プロトタイプ UI の実装
    • 探索的な段階からスタートしているので PR が大きめです
    • Remix で素朴な SSR をしています
    • コロケーションなども不完全ですが、あとからリファクタ + テスト追加を同時に行います

TODO

  • サブドメイン紐づけ後対応
  • アクセシビリティ対応
    • input
    • コントラスト比
      • Mantine の Light Theme が全部だめかも知れない
  • 現在の検索条件の共有
    • URL をクリップボードにいれるだけで良い
  • ページネーションの仮ロジック削除
  • ライブラリ自動更新の設定
    • dependabot, renovate など
  • TailwindCSS と Mantine で微妙にスタイルが違っている部分の吸収

スクリーンショット

PC

フルページ (別タブでの閲覧を推奨) 簡易検索 詳細検索
localhost_5173__note_status=CURRENTLY_RATED_HELPFUL offset=0 limit=2 スクリーンショット 2025-02-05 162634 スクリーンショット 2025-02-12 154202

Mobile

フルページ (別タブでの閲覧を推奨) 簡易検索 詳細検索
localhost_5173__note_status=CURRENTLY_RATED_HELPFUL limit=25 offset=0(iPhone SE) スクリーンショット 2025-02-12 154247 スクリーンショット 2025-02-12 161126

|

@sushichan044 sushichan044 self-assigned this Feb 12, 2025
<MantineTextInput
autoComplete={autoComplete}
inputWrapperOrder={mantineInputOrder}
{...(autoComplete === "off" && { "data-1p-ignore": true })}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

1password が必要以上に自動補完してくるのを防ぐ

const shortLanguage = useLanguageLiteral("ja");

const navigation = useNavigation();
const searchInProgress = navigation.state !== "idle";
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

idle → (submit) → submitting → (execute action) → loading → (execute loader) → idle
なので、idle でない場合は常に何らかのロードを行っているとみなす

@@ -0,0 +1,50 @@
import { parseURL, stringifyParsedURL, withQuery } from "ufo";
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

API 側のページネーションが既に治っているので必要ない実装だが、あとから消すことにした

formType,
...rest
}: UseNoteSearchFormOptions): UseNoteSearchFormReturn => {
const formIdHash = useMemo(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

defaultValue が変わると必ず formIdHash の値が変わる。

これを useForm の id に渡してあげることで、
defaultValue が変わったらすべての input を明示的に再レンダリングできる

@@ -12,7 +12,7 @@ export interface Note {
/**
* コミュニティノートの作成日時 (ミリ秒単位の UNIX EPOCH TIMESTAMP)
* @minimum 1152921600000
* @maximum 1736905314223
* @maximum 1738808503163
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

このあたりの値が定数になるのは予想していない挙動だと思うので、
OpenAPI Spec の修正が必要

{
rel: "canonical",
// TODO: change before production
href: "https://birdxplorer.code4japan.org",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

これが確定するまでにリリースすると危険

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

先に noindex nofollow でデプロイして、ドメイン紐づけが完了次第 noindex nofollow を外すことで合意済み

Comment on lines +54 to +73
// 簡易検索画面においても詳細検索にしかない検索条件の値は保持される。
// このため、簡易検索の条件を入れて検索したつもりでも、詳細検索にしかない条件が追加で入る可能性がある。
// これは、詳細検索を多用するユーザーには利用しやすいが、簡易検索を多用するユーザーには予期せぬ挙動となる。
// この問題を解決するために、詳細検索のみの条件が指定されている場合はその数をユーザーにフィードバックしたい。
const hiddenInputKeys = Object.keys(form.value ?? {}).filter((key) => {
return !(
[
"note_includes_text",
"topic_ids",
"language",
"note_created_at_from",
"note_created_at_to",
"limit",
"offset", // フォームでは入力しないが、ページネーションは簡易検索の画面に表示されているので
] satisfies Array<keyof NoteSearchParams>
).includes(
// @ts-expect-error ここでは型が合わないが、ランタイムの挙動は Literal[] と string の比較になるので問題ないstring
key,
);
});
Copy link
Collaborator Author

@sushichan044 sushichan044 Feb 12, 2025

Choose a reason for hiding this comment

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

コメントにも書いているが、

  • 詳細検索限定の検索パラメータは簡易検索画面でも保持される
    • 詳細検索を多用するユーザーに都合が良い挙動
    • 問題: 簡易検索のパラメータのみ指定したつもりでも詳細検索のパラメータが含まれる挙動が出る

この問題に対処するために詳細検索限定の条件が何個指定されているか計算し表示している

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

このロジックは後で切り出してテストを書くべき

@sushichan044 sushichan044 marked this pull request as ready for review February 12, 2025 07:54
@yu23ki14 yu23ki14 merged commit ce15d14 into main Feb 12, 2025
1 check passed
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.

2 participants