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

traPブログへのリンクを追加 #147

Merged
merged 1 commit into from
Jun 11, 2024
Merged

Conversation

mehm8128
Copy link
Contributor

close #136

@mehm8128 mehm8128 requested a review from Pugma June 10, 2024 12:47
@mehm8128 mehm8128 self-assigned this Jun 10, 2024
Copy link
Collaborator

@Pugma Pugma left a comment

Choose a reason for hiding this comment

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

今回のPRの内容としてはこの実装で問題ないと思うんですが、他のところとの兼ね合いでちょっと気になりました
ご確認お願いします

target="_blank"
rel="noreferrer noopener"
>
<img src="/@/assets/traP_logo_icon.svg" width="24" height="24" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

ここでblogだけ別個で実装したのはsvgファイルを別個で用意しているからですかね…?
これで行くとAtCoderのロゴもこういう実装になるんですかね

Copy link
Contributor Author

Choose a reason for hiding this comment

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

そうですね、ここだけ別です
AtCoderも確かに今のままでは無理ですが、AccountListItemコンポーネント自体の実装を修正すれば多分いい感じにsvgにもアイコンにも対応できるようにはできそうです(まだロゴ用意できてなさそうなので今はこのままで)

Copy link
Collaborator

Choose a reason for hiding this comment

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

あ〜、だとするとAtCoderのsvgが用意できた状態でAccountListItemの実装を直す感じになるんですかね…?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

そうなりそうですね

Copy link
Collaborator

@Pugma Pugma left a comment

Choose a reason for hiding this comment

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

一旦svgロゴをどう表示するかについては議論ができたのでこのPRはここでマージしちゃいましょう!
ありがとうございました

@mehm8128 mehm8128 merged commit c4668fa into master Jun 11, 2024
8 checks passed
@mehm8128 mehm8128 deleted the feat/add_traP_blog_url branch June 11, 2024 12:58
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.

traPブログへのリンクを表示する
2 participants