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

スポンサー関連のデータを追加 #176

Merged
merged 16 commits into from
Sep 25, 2023

Conversation

tatsutakein
Copy link
Contributor

@tatsutakein tatsutakein commented Sep 24, 2023

Issue

Overview (Required)

スポンサー関連のデータを追加して、スポンサーロゴカードの背景を黒に変更しました。

CyberAgent, harmo, RECRUIT のロゴは flutter_svg では非対応の形式で出力されてしまっていたため、はじめに各企業からいただいていた png 形式で表示するようにしました。

Screenshot

image

@github-actions github-actions bot added the feature New feature or request label Sep 24, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 24, 2023

Visit the preview URL for this PR (updated for commit cf7a1b8):

https://flutterkaigi-2023-preview--pr176-feature-add-sponsor-cvaa8go8.web.app

(expires Mon, 02 Oct 2023 06:13:59 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: ec86c184da0140e1a174c92c6d39f7a77720a028

@tatsutakein tatsutakein marked this pull request as ready for review September 24, 2023 14:22
Copy link
Contributor

@YumNumm YumNumm left a comment

Choose a reason for hiding this comment

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

@tatsutakein
2点 コメントさせて頂きました🙏 ご確認お願い致します!

Comment on lines 14 to 20
Future<List<Sponsor>> fetchSponsors() async {
return [
..._platinumSponsors,
..._goldSponsors,
..._silverSponsors,
];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

ask-badge
こちら、Future<List<Sponsor>>ではなくList<Sponsor>の方が良さそうですが、FutureProviderにしている意図ありますでしょうか?


imo-badge
もし同期関数で良いのであれば、Providerの方が扱いやすいかもしれません!

@Riverpod(keepAlive: true)
List<Sponsor> sponsor(...) => [...];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

仰るとおり、同期関数のため普通に List<Sponsor> で良さそうでした!
e5b026c にて対応いたしました。

plan: SponsorPlan.gold,
session: null,
introduction:
'10Xではモバイル・WebアプリでFlutterを、サーバーサイドではDartを利用しています。このように10XはFlutter・Dartに支えられており、今後も価値あるプロダクトを作り続ける上で重要な技術への支援・還元を行っていきます。',
Copy link
Contributor

Choose a reason for hiding this comment

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

GitHub Actions/analyze

The line length exceeds the 80-character limit.

Try breaking the line across multiple lines.
See https://dart.dev/lints/lines_longer_than_80_chars to learn more about this problem.

80文字制限について、ファイル全体でignoreしてしまって良さそうです!

https://github.com/FlutterKaigi/2023/actions/runs/6290584624?pr=176#:~:text=by%20humans.txt-,Annotations,-4%20warnings%20and

Copy link
Contributor Author

Choose a reason for hiding this comment

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

おお、ありがとうございます…!
960b07f にて対応いたしました。

@tatsutakein
Copy link
Contributor Author

@YumNumm
いただいたフィードバックについて対応いたしました。
お手数ですが、再度ご確認いただけますでしょうか 🙏

@tatsutakein tatsutakein requested a review from YumNumm September 25, 2023 00:21
@blendthink blendthink force-pushed the feature/add-sponsor-data branch from d7060f5 to ad1f56b Compare September 25, 2023 04:19
@blendthink blendthink force-pushed the feature/add-sponsor-data branch from ad1f56b to 270e03e Compare September 25, 2023 04:33
@blendthink
Copy link
Contributor

@YumNumm @tatsutakein
こちら Slack でご連絡させていただきましたが、いったんマージさせていただきます、、!
何かあれば別プルリクで対応します!

@blendthink blendthink merged commit 38d0fb5 into feature/sponsor Sep 25, 2023
@blendthink blendthink deleted the feature/add-sponsor-data branch September 25, 2023 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants