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

AlertDialogのUI実装 #19

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
Open

AlertDialogのUI実装 #19

wants to merge 12 commits into from

Conversation

KobayashiYoh
Copy link
Collaborator

@KobayashiYoh KobayashiYoh commented Feb 28, 2024

概要

Alert DialogのUIコンポーネントとその表示を行うメソッドを実装しました。
Alert DialogのデザインをなるべくFlutter標準のものに近づけるため、UI実装したものをデザインに逆輸入する予定です。

UI(Android)

UI(iOS)

@KobayashiYoh KobayashiYoh self-assigned this Feb 28, 2024
@KobayashiYoh KobayashiYoh changed the title Feature/alert dialog AlertDialogのUI実装 Feb 28, 2024
@KobayashiYoh KobayashiYoh marked this pull request as ready for review February 28, 2024 03:13
Copy link
Collaborator

@Ryunosuke1114 Ryunosuke1114 left a comment

Choose a reason for hiding this comment

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

実装ありがとう!コメントしました!

Copy link
Collaborator

Choose a reason for hiding this comment

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

このコードってどこかで使用される予定とか将来的にあったりする?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Figmaにはまだ書いてないけどログアウトとか権限周りで使うよ

_button(
'Alert Dialog',
onPressed: () {
Navigator.of(context).push(
Copy link
Collaborator

Choose a reason for hiding this comment

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

pushだけUtilのメソッドに変更して欲しい🙏

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

修正しやす🙆‍♂️

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

d29fed3
修正した

import 'package:pg_mobile/widgets/common_cupertino_alert_dialog.dart';
import 'package:pg_mobile/widgets/common_material_alert_dialog.dart';

class NavigatorUtil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

この名前だとrooting系の処理まとまってるクラスなのかなーって思ってしまうから
OSでモーダルの処理を分けてる旨が伝わる命名がベターだと思った
PlatformModalHandlerとかOSModalDispatcherとか

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

確かにそうやね…
クラス名をPlatformModalHandlerに変更するね

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

c991d8e
修正した

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.

2 participants