-
Notifications
You must be signed in to change notification settings - Fork 6
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
Responsive Builder #19
Conversation
import 'package:flutter/material.dart'; | ||
|
||
class ResponsiveHelper { | ||
static const double tabletMinWidth = 800; |
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.
Where did you get this code? From Android, I remember this value was 720.
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.
It can change.
MediaQuery.of(context).size.width < desktopMinWidth; | ||
|
||
static bool isDesktop(BuildContext context) => | ||
MediaQuery.of(context).size.width >= desktopMinWidth; |
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 am not sure what this does. How can a width be related to desktop?
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.
It checks the screen width of the device. If the width is good for our desktop width, it returns true.
lib/widgets/responsive_builder.dart
Outdated
@required this.mobile, | ||
this.tablet, | ||
@required this.desktop, | ||
}) : assert(mobile != null, "Mobile cannot be empty."), |
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.
Let's use single quotes for texts.
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.
lib/widgets/responsive_builder.dart
Outdated
final Widget tablet; | ||
|
||
/// 1200px | ||
final Widget desktop; |
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.
Why we have a difference between desktop and tablets. An ipad can be as big as a laptop for example.
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 tried to use the standards like Bootstrap, Microsoft. Generally, sources use three different devices. Example
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.
Yes but that is not what we do with Flutter. We care about only the size. So let's try to keep it as small medium large etc.
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.
What is your suggestion? Should we remove the helper?
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.
We should rename them to something more generic. Because a desktop can be as small as a tablet and a tablet can be as big as a Desktop
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.
We can change the names to Small, Medium, Large.
Screen Names were updated.
Fixes #18