-
Notifications
You must be signed in to change notification settings - Fork 19
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
UI/connectivity warning #1412
base: develop
Are you sure you want to change the base?
UI/connectivity warning #1412
Conversation
I like the design. If the pharse was rephase to "Attention: No internet connection" would be better than demanding an action from the user. The user may be checking his next class, local data will be sufficient and no internet connection is required. |
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 a lot the option without background and red text on light and white on dark.
packages/uni_app/lib/view/common_widgets/connectivity_warning_card.dart
Outdated
Show resolved
Hide resolved
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.
One last request:
Don't upload an asset image to use as an icon. Import from the icon package Uni is using
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.
Well done guys!
Don't forget to make the actions go green!
@@ -29,7 +30,10 @@ class AcademicPathPageViewState extends GeneralPageViewState { | |||
@override | |||
Widget getBody(BuildContext context) { | |||
return ListView( | |||
children: academicPathCards, | |||
children: [ | |||
const ConnectivityWarning(), |
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.
To make the position of the ConnectivityWarning fixed
, move it to the getTopNavBar
method in GeneralPageViewState
(however, note that the layout of the pages may change during the redesign).
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.
Following the conversation we had, I tried to change the _createTopWodgets
method in the top_navigation_bar.dart
class but there are 2 problems. The first is that the home page uses a getHeader method that creates the tittle differently from the other pages (the warning appears above instead of below the tittle). The second problem is that the size of the AppBar isnt dynamically changed when the warning is displayed, so it seems cut out. Do I have to increase its size or its my implementation that isnt correct?
Closes #528
//
Review checklist
whatsnew/whatsnew-pt-PT
changelog.md
with the change