Skip to content
This repository was archived by the owner on Jun 1, 2023. It is now read-only.

Conversation

@yamanoku
Copy link
Contributor

@yamanoku yamanoku commented Jun 19, 2021

👏 解決する issue / Resolved Issues

⛏ 変更内容 / Details of Changes

  • TOP上部にあるお知らせリンク箇所で日付箇所がリンクに含まれていたため分離

📸 スクリーンショット / Screenshots

変更後のスタイルスクリーンショット。本番環境と差異がないことを確認願います。
デスクトップビュー

スマートフォンビュー

yamanoku added 2 commits June 19, 2021 21:12
見た目とマークアップの齟齬修正

日付をリンク箇所から分離

dl > dt + dd でマークアップ
</li>
</ul>
</div>
<ul class="WhatsNew-list">
Copy link
Collaborator

@kaizumaki kaizumaki Jun 20, 2021

Choose a reason for hiding this comment

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

たぶんですね、この uldl にするということだと思うのです。 ul li の中に dl があるのはここの場合はセマンティックが過剰に行われている気がするのですが、いかがでしょう?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

レビューありがとうございます。

ul li の中に dl があるのはここの場合はセマンティックが過剰に行われている気がするのですが、いかがでしょう?

Screen.Recording.2021-06-20.at.22.42.39.mov

リストマークアップをしておくとそのリスト内部が何項目あるかを読み上げてくれるので、そちらの形は以前のまま尊重した形になっています。

Copy link
Collaborator

Choose a reason for hiding this comment

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

おお、なるほど、そうなのですね!であれば、 dl のマークアップまではしなくていいような気がするんですけど、どうなんでしょうね...?
ちょっと、私では判断できず...。@magi1125 さん、レビューお願いできますでしょうか?

Copy link

@magi1125 magi1125 Jun 21, 2021

Choose a reason for hiding this comment

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

@kaizumaki @yamanoku むずかしいところですねー、私の意見で言うと…

  • 日付にリンクが掛かっている現状は視覚と読み上げが分離していて改善したほうが良いので、なにしろ要素は分けたほうが良い
  • AS情報を見るに、dlもリストと言うので、この実装だとリスト内リストになり、環境によっては「リスト5項目」に「リスト1項目」がある格好になりそう。
  • リストの項目数を言うかはスクリーンリーダー次第。iOS VOだと「説明リストの始まり〜説明リストの終わり」とだけ言い、項目数は言わなかった。Android Talkbackだと、dd要素の読み上げ末尾で「定義」と言うに留まっていた。
  • なので、セマンティクスとして提案の実装が問題があるわけではない(HTMLに存在するセマンティクスでなるべく近いものを使って厳密に紐付けるならこの実装になる)が、スクリーンリーダーの解釈の状況を考えると別案でも良いかもという気がする。
  • 案としては ①uldlに置き換える、②ulは残して日付とリンクテキストをspanで分ける、③いまの案で行く、のどれか。自分ならたぶん②を選ぶ。①はリスト項目数がわからなくなるが、そこまで項目数と内容が多くないので問題なさそう。②は日付と内容のセマンティクス上の紐付けができなくなるが、同一のli内にあることでひも付きはわかるし、日付と内容を要素的に分離するというお題は果たせている。
  • ③の現状実装でも致命的にアクセシブルじゃなくなるわけではない。おそらく慣れればわかるレベル。
  • いちおう 「④ulは残しつつむしろ日付まで全部リンクのスタイルにしちゃう」というアプローチもある

という感じでしょうか。

Copy link
Collaborator

Choose a reason for hiding this comment

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

@magi1125 コメントありがとうございます!詳細な解説、感謝です 🙏
ご提示いただいた案の中では、私も2が自然のような気がしますね。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@magi1125 コメントありがとうございます。モバイルでのスクリーンリーダー読み上げは失念していました。

①については b8058ca (#6446) をリバートすれば実現できますが、
②の方針で自分も納得したのでそちらで修正進めたいと思います 👍🏼

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kaizumaki @magi1125
124f2c5 (#6446)にて対応いたしました。

}
&-ExternalLinkIcon {
.ExternalLinkIcon {
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

@kaizumaki kaizumaki left a comment

Choose a reason for hiding this comment

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

ご対応ありがとうございます!よいと思います!

@kaizumaki kaizumaki requested a review from magi1125 June 21, 2021 04:06
Copy link

@magi1125 magi1125 left a comment

Choose a reason for hiding this comment

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

LGTM!

@kaizumaki kaizumaki added the ready-for-merge コードレビューが終わり、マージができるもの label Jun 21, 2021
@soutaito soutaito merged commit dd30fa7 into Tokyo-Metro-Gov:development Jun 24, 2021
@yamanoku yamanoku deleted the fix-whatsnew-list-markup branch June 25, 2021 02:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

ready-for-merge コードレビューが終わり、マージができるもの

Projects

None yet

Development

Successfully merging this pull request may close these issues.

最新のお知らせのリンクリストに見た目とマークアップの齟齬がある

4 participants