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

[Feature Request]: OGPの取得を手動化したい #31

Closed
nkte8 opened this issue Mar 12, 2024 · 8 comments
Closed

[Feature Request]: OGPの取得を手動化したい #31

nkte8 opened this issue Mar 12, 2024 · 8 comments
Assignees
Labels
enhancement New feature or request Essential High Priority Issue

Comments

@nkte8
Copy link
Owner

nkte8 commented Mar 12, 2024

Request Details

リンクカードが取得できない、想定と違うポストになってしまう、という問題の本質として、そもそもOGPカードに対応しているしていないかの判断がポスト前にできないことにあると思う。
Blueskyアプリや、他サードパーティクライアントと同様にリンクカードの取得自体が可能かどうかをポスト前に判断できれば、投稿内容が想定と不一致であるという部分でのミスマッチを抑えることができるのではないかと考える。

Feature Scope

Usability increase, user satisfication (ユーザビリティ向上させ、利用者を満足させる)

Feature Add infomation

No response

@nkte8 nkte8 added the enhancement New feature or request label Mar 12, 2024
@nkte8 nkte8 self-assigned this Mar 12, 2024
@nkte8 nkte8 transferred this issue from nkte8/skyshare-archive Mar 13, 2024
@ZEKE320
Copy link
Collaborator

ZEKE320 commented Mar 16, 2024

@nkte8
ここのIssueですが、Bluesky側で「URLと画像を同時設定した際にURLのリンクカード(OGP)が優先されてしまう」事象が報告されているようなので、早めに着手したほうが良いかもしれませんね

今すぐ対応できそうな例としては、単純に

  • 添付画像がある場合はURLのリンクカードより優先して投稿に加える
    というもので良いような気がしてます

ゆくゆくはOGP取得が1投稿につき2重になっている問題の解決と合わせて、

  • 投稿画面にリンクカードのプレビュー機能を実装する
  • リンクカードと画像による投稿を切り替えるボタンを追加する
    という進め方もアリかもしれません

@nkte8
Copy link
Owner Author

nkte8 commented Mar 16, 2024

@ZEKE320 仰っている通り、今上がっている報告事例の対応もこのIssueで解決できますね。
ただ、今上がっているPRなどでimageの部分の実装とかリファクタリングというか実装の改善をしていただいていると思っていて、変更の影響範囲を把握しきれていないので、少なくともOGP取得の手動化対応は少なくとも #40 が吐けたらかなぁと思っています。 #40 の内容を把握してから影響ない範囲で対応できればいいのですが、稼働が取れず...
(私はこの辺が怖いのでPRによる変更は1PR1機能に絞るべきだと思っているのですが、まだその辺のルールは自分自身についても決めていないので、しょうがないって感じです。)

@ZEKE320
Copy link
Collaborator

ZEKE320 commented Mar 16, 2024

@nkte8
そうですね。了解です。

[以下、問題に関する補足情報です]
問題のコードを調査・検証した限りでは一応、リンクカードよりも画像が優先されている様な作りにはなっていますね。
何かしらのバグが原因で、リンクカード付きの投稿が残るような不具合が起きているのではないかなと
開発環境で再現できれば手の打ちようがあるのですが、今のところは再現できていません

@nkte8
Copy link
Owner Author

nkte8 commented Mar 16, 2024

ですよね...リンクカードの生成するかって以下のようになっていて、noImagesAttached = (imageFiles.length <= 0)なはずなので、なんで起きてるんだろ...というきもち。

                    if (noGenerate || noImagesAttached) {  <----------ここで弾かれてるはず
                        record = await attachExternalToRecord({
                            apiUrl: siteurl,
                            base: record,
                            session: sessionNecessary,
                            externalUrl: new URL(linkcardUrl),
                            handleProcessing: setMsgInfo
                        })
                    }

他のOGPの話もあるから、このIssueでの改善で全部まとめて解決しちゃうのがいい、はねこのも同感です。

@nkte8 nkte8 added the Essential High Priority Issue label Mar 16, 2024
@nkte8
Copy link
Owner Author

nkte8 commented Mar 17, 2024

じゃじゃっと実装中
画像のプレビューを司っている部分に外部リンクのプレビューを表示したかったので、プレビューの部分の構造を変えようとしています。MediaDataコンストラクタをuseStateで定義

    // メディアのプレビューに関するStateコンストラクタ
    const [mediaDataList, setMediaDataList] = useState<Array<MediaData>>([])

こんな感じの情報を持っている型です。

import { ogpMetaData } from "@/lib/types"
export type MediaData = LinkCard | Image

type LinkCard = {
    type: "linkcard",
    blob: Blob | null,
    meta: ogpMetaData & {
        url: string
    }
}
type Image = {
    type: "image",
    alt: string,
    blob: Blob | null,
}

コンポーネントの作業範囲も明確にするため、PostFormではなくリンクカードを手動取得するボタンの方にメソッドを仕込んでいます。(LinkcardAttachButton.tsxから引用)

export const Component = ({
    postText,
    setMediaDataList,
...
    const handlePost = async () => {
        const richTextLinkParser = new richTextFacetParser("link")
        const parseResult = richTextLinkParser.getFacet(postText)
        if (parseResult.length < 1) {
            return
        }
        setProcessing(true)
        setMsgInfo({
            isError: false,
            msg: `リンクカードを取得中...`
        })
...
    return (
        <ProcButton
            handler={handlePost}
            isProcessing={isProcessing}
            context="リンクカードを取得"
            showAnimation={showAnimation}
            className={["mx-1"].join(" ")}
            disabled={disabled} />
    )

作業範囲の明確化について、本当はhandlePost(ポストを実際に行うハンドラー)もPostFormに持たせるのではなく、PostButtonのコンポーネントに持たせたいなぁという気持ちがありつつ、このissueでそこまでやるのは破壊的すぎるので、imageを統合する部分あたりまでが限界かなって感じ。コード改善としてこの辺は一旦起票してから進めます。

@nkte8
Copy link
Owner Author

nkte8 commented Mar 17, 2024

...と思ったけど、OGP手動取得の時点でこれまで使ってたrecordBuilderの一部が重複実行になってしまうから、結局この辺もいじることになるし、それならプロトタイプとして既存のコードを変えない形で実装だけしちゃおう。Postボタン(旧)とPostボタン(新)をレンダリング箇所の書き換えだけで切り替えてつかえるイメージ。
実際今handlePostに依存している箇所も、コンポーネントから関数引っ張ってくれば大丈夫そうだし、デグレだけ気をつけて進めてみます

@nkte8
Copy link
Owner Author

nkte8 commented Mar 17, 2024

imageFiles変数を使わない作りになったことから、これまでこれありきで動作していた箇所を全て置き換えを行う必要がある。PostButtonコンポーネントについてはPostFormから切り出す形で対応できたが、PopupPreviewFormコンポーネントについては複製を作るわけにもいかない(基本的にコンポーネントレベルでのhogehoge_v2といった実装は好まれない)ため、やっぱり破壊的変更になりそう。

現在の状態については https://github.com/nkte8/skyshare/tree/feature/issue_31 にて確認ができます。

...ので、さっさと作り上げます。本issue解決によりOGP画像はStateとして保存されるため現在のような再取得も必要なく、 #19 も同時に解消される見込みです。

@nkte8 nkte8 mentioned this issue Mar 20, 2024
nkte8 added a commit that referenced this issue Mar 20, 2024
@nkte8
Copy link
Owner Author

nkte8 commented Mar 20, 2024

v1.4.0 にて対応完了

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Essential High Priority Issue
Projects
None yet
Development

No branches or pull requests

2 participants