-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Feat/open buildings from contact #150
Conversation
f6fe9da
to
ce48c25
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me :-) i especially like the new way of launching url utils
with help of ref
.
I left only one note regarding minor change.
return GestureDetector( | ||
onTap: () async { | ||
await LaunchUrlUtil.launch(url); | ||
await ref.launch(url); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use here unawaited
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, but also there's nothing after it, so
onTap: () async => ref.launch(url),
should have similar effect I believe
import "../utils/where_non_null_iterable.dart"; | ||
|
||
extension LaunchUrlUtilX on WidgetRef? { | ||
Future<bool> launch(String uriStr) async { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the change here. Universal in all places rn.
ce48c25
to
d2842fe
Compare
I've been fixing some issues that occured during filling the info in the backend and done the opening links as well (haven't seen your assignemnet @mikolaj-jalocha ) Upsik
plac
topl.
to be consistent with buildings3.24.0