vuejs-jp / vuefes-2019

29 stars 5 forks source link

feat: タイムテーブル情報を Contentful から取得&描画するよう変更 #177

Closed inouetakuya closed 5 years ago

inouetakuya commented 5 years ago

feat: タイムテーブル情報を Contentful から取得&描画するよう変更 by aytdm · Pull Request #168 · kazupon/vuefes-2019 のリベンジ。

resolve: タイムテーブルの内容を埋める · Issue #136 · kazupon/vuefes-2019

デザイン

https://www.figma.com/file/BpIgcZdusbS3CgPHQhhK79T7/Vue-Fes-Japan-2019-Web?node-id=2%3A4

スケジュール

9/18(水)にレビュー依頼をして、なる早で公開したい。

TODO

レビューポイント

(1) タイムテーブルに関するデータを Vuex Store で保持することについて

(2) Contentful API から取得したサンプルデータ(という名の、本番データと同じ内容のデータ)の保存場所

(3) Vuex Store の各モジュールに関する型定義の保存場所

これまで src/store/foo.ts の中に型定義を書いて、他の場所でも import して使っていたが、再利用する型定義は src/types/ 配下にまとめた

(4) コンポーネントの分割

これはもっと早い段階で、ガイドラインをドキュメントにして、チーム内で検討し、共通の認識を持てるようにすべきだった。反省。

今回のプルリクエストの範囲では、下記のようにしています。

余談だけど、Vuex Store にアクセスできるか否かと、レイアウトに責務を持つか否かについて、コンポーネント名とディレクトリによって、分かるようにしたいと考えていて、Vue Fes Japan 2019 サイトでは見送るけど、どこかで発表するなりしたい。

(5) キーノートの同時通訳あり表示

あったほうが良いと思ったので表示させてみたが、どうでしょう?

など。

(6) デザイン

デザインを正しく表現できているか

(7) その他

コーディングで気になる点はないか

参考

inouetakuya commented 5 years ago

@hisako135 CC: @448jp

(5) キーノートの同時通訳あり表示

も含めて、デザインのレビューをお願いします!:pray:

なお https://github.com/kazupon/vuefes-2019/issues/136#issuecomment-530942894 にある、会場転換とアフターパーティーを区切る件については、Contentful の調整等が必要であるため、別 PR にさせてください 🙏


エンジニアの皆さん

ファイル差分及びレビューポイントが多いですが、タイムテーブルは参加者にとって、非常に有用な情報ですので、できるだけ早く公開したいと考えています。急かしてしまい恐縮ですが、なる早のレビューをお願いします 🙏

inouetakuya commented 5 years ago

@ryamakuchi

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

(2) Contentful API から取得したサンプルデータ(という名の、本番データと同じ内容のデータ)の保存場所

@aytdm が作成してくださった https://github.com/kazupon/vuefes-2019/pull/168 と同じやり方で、JSON を {{ myJson }} というかたちでビューに埋め込みました。詳細はたぶん ↑ の PR のソースを見ると分かると思いますー。

カンプと相違がある部分

https://github.com/kazupon/vuefes-2019/pull/117#discussion_r301401176 https://github.com/kazupon/vuefes-2019/pull/117#discussion_r301401257

こういう経緯でして、今回の PR 以前からの仕様ですね。

inouetakuya commented 5 years ago

@hisako135 @448jp

クローズについて

(1) lg のアフターパーティー時の PLAID ルーム以外の枠について

Contentful のデータを使うとこれらを合体させることは厳しいので、合体させようとすると、この部分だけ個別にマークアップすることになります。なんか見た感じ、現状の分かれたままでも良さそうと思ったのですが、どうでしょう?

(2) 背景色について

background-color: rgba(255, 255, 255, 0.6);

image

background-color: rgba(255, 255, 255, 0.5);

image

結局 background-color: rgba(255, 255, 255, 0.5); を採用してみましたが、どうでしょう?

inouetakuya commented 5 years ago

皆さん、レビューありがとうございます!指摘いただいた箇所について修正しましたので確認お願いします〜。


@hisako135

公開前にひさこさんのレビューを受けておきたいので、よろしくお願いします 🙏

可能であれば 9/22(日)に公開したい気持ちです :bow:

https://deploy-preview-177--vuefes-2019.netlify.com/

448jp commented 5 years ago

@inouetakuya (1) (2) ともに井上さんのご判断を支持いたします! 1点だけ、昨日のミーティングでコミュニティセッションの内容について原稿を直すので、そこだけ再コメントしますが、それ以外はLGTMです。

inouetakuya commented 5 years ago

@hisako135

タブレットとスマホサイズの時のフォントサイズ、もう少し大きくして良さそう

たしかに。対応しましたので、念のため確認お願いします〜 🙏

inouetakuya commented 5 years ago

皆さん、レビューありがとうございました!マージします!!:octocat:

aytdm commented 5 years ago

ご対応ありがとうございましたmm