Skip to content

[Gallery] Preserve demo state, simplify demo width calculations on desktop #287

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jan 28, 2020
Merged

[Gallery] Preserve demo state, simplify demo width calculations on desktop #287

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1493cf0
Maintain state on mobile
guidezpl Jan 28, 2020
f0278d8
Refactor section width code
guidezpl Jan 28, 2020
1478f36
Fix state resetting on desktop
guidezpl Jan 28, 2020
e055a9a
Remove unused import
guidezpl Jan 28, 2020
5182954
Remove unecessary GestureDetectorBehavior
guidezpl Jan 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
217 changes: 100 additions & 117 deletions gallery/gallery/lib/pages/demo.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// found in the LICENSE file.

import 'dart:io' show Platform;
import 'dart:math';

import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
Expand Down Expand Up @@ -256,12 +255,14 @@ class _DemoPageState extends State<DemoPage> with TickerProviderStateMixin {
appBar.preferredSize.height;
final maxSectionHeight = isDesktop ? contentHeight : contentHeight - 64;
final horizontalPadding = isDesktop ? mediaQuery.size.width * 0.12 : 0.0;
final maxSectionWidth = 420.0;

Widget section;
switch (_state) {
case _DemoState.options:
section = _DemoSectionOptions(
maxHeight: maxSectionHeight,
maxWidth: maxSectionWidth,
configurations: widget.demo.configurations,
configIndex: _configIndex,
onConfigChanged: (index) {
Expand All @@ -277,6 +278,7 @@ class _DemoPageState extends State<DemoPage> with TickerProviderStateMixin {
case _DemoState.info:
section = _DemoSectionInfo(
maxHeight: maxSectionHeight,
maxWidth: maxSectionWidth,
title: _currentConfig.title,
description: _currentConfig.description,
);
Expand Down Expand Up @@ -314,46 +316,15 @@ class _DemoPageState extends State<DemoPage> with TickerProviderStateMixin {
buildRoute: _currentConfig.buildRoute,
);
if (isDesktop) {
// If the available width is not very wide, reduce the amount of space
// between the demo content and the selected section.
const reducedMiddleSpaceWidth = 48.0;

// Width of the space between the section and the demo content
// when the code is NOT displayed.
final nonCodePageMiddleSpaceWidth = mediaQuery.size.width > 900
? horizontalPadding
: reducedMiddleSpaceWidth;

// Width of the space between the section and the demo content
// when the code is displayed.
final codePageMiddleSpaceWidth =
min(reducedMiddleSpaceWidth, nonCodePageMiddleSpaceWidth);

// Width of the space between the section and the demo content
final middleSpaceWidth = _state == _DemoState.code
? codePageMiddleSpaceWidth
: nonCodePageMiddleSpaceWidth;

// Width for demo content.
// It is calculated in this way because the code demands more space.
final demoContentWidth = (mediaQuery.size.width -
horizontalPadding * 2 -
nonCodePageMiddleSpaceWidth) /
2;

final Widget sectionAndDemo = (_state == _DemoState.fullscreen)
? demoContent
: Row(
crossAxisAlignment: CrossAxisAlignment.start,
children: [
Expanded(child: section),
SizedBox(width: middleSpaceWidth),
Container(
width: demoContentWidth,
child: demoContent,
),
],
);
final isFullScreen = _state == _DemoState.fullscreen;
final Widget sectionAndDemo = Row(
crossAxisAlignment: CrossAxisAlignment.start,
children: [
if (!isFullScreen) Expanded(child: section),
SizedBox(width: !isFullScreen ? 48.0 : 0),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might be easier to read as:

if (!isFullScreen) ...[
  Expanded(child: section),
  SizedBox(),
]
Expanded(child: demoContent),

Copy link
Member Author

@guidezpl guidezpl Jan 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This is what I was saying causes the demoContent to be rebuilt from scratch. I could write it as

if (!isFullScreen) ...[
  Expanded(child: section),
  SizedBox(width: 48.0),
]
SizedBox(),
Expanded(child: demoContent),

and it works. It's puzzling

Copy link
Contributor

Choose a reason for hiding this comment

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

Very strange...ok I think I prefer what you originally had instead of the 2nd sizedbox

Expanded(child: demoContent),
],
);

body = SafeArea(
child: Padding(
Expand All @@ -371,20 +342,22 @@ class _DemoPageState extends State<DemoPage> with TickerProviderStateMixin {
);

// Add a tap gesture to collapse the currently opened section.
if (_state != _DemoState.normal) {
demoContent = Semantics(
label: MaterialLocalizations.of(context).modalBarrierDismissLabel,
child: GestureDetector(
behavior: HitTestBehavior.opaque,
onTap: () {
demoContent = Semantics(
label: MaterialLocalizations.of(context).modalBarrierDismissLabel,
child: GestureDetector(
onTap: () {
if (_state != _DemoState.normal) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the gestureDetector is added always now, does this interfere with interacting with the demo?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've tested on chrome, iOS and macOS, and saw no adverse effects. I'll remove behavior: HitTestBehavior.opaque because it's not doing anything here and might be confusing

setStateAndUpdate(() {
_state = _DemoState.normal;
});
},
child: ExcludeSemantics(child: demoContent),
}
},
child: Semantics(
excludeSemantics: _state != _DemoState.normal,
child: demoContent,
),
);
}
),
);

body = SafeArea(
bottom: false,
Expand Down Expand Up @@ -491,12 +464,14 @@ class _DemoSectionOptions extends StatelessWidget {
const _DemoSectionOptions({
Key key,
this.maxHeight,
this.maxWidth,
this.configurations,
this.configIndex,
this.onConfigChanged,
}) : super(key: key);

final double maxHeight;
final double maxWidth;
final List<GalleryDemoConfiguration> configurations;
final int configIndex;
final ValueChanged<int> onConfigChanged;
Expand All @@ -506,49 +481,52 @@ class _DemoSectionOptions extends StatelessWidget {
final textTheme = Theme.of(context).textTheme;
final colorScheme = Theme.of(context).colorScheme;

return Container(
constraints: BoxConstraints(maxHeight: maxHeight),
child: Column(
crossAxisAlignment: CrossAxisAlignment.start,
mainAxisSize: MainAxisSize.min,
children: [
Padding(
padding: const EdgeInsetsDirectional.only(
start: 24,
top: 12,
end: 24,
),
child: Text(
GalleryLocalizations.of(context).demoOptionsTooltip,
style: textTheme.display1.apply(
color: colorScheme.onSurface,
fontSizeDelta:
isDisplayDesktop(context) ? desktopDisplay1FontDelta : 0,
return Align(
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the Align for?

Copy link
Member Author

Choose a reason for hiding this comment

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

A Container within an Expanded will fill the space, even if you set width/height constraints, unless you use an Align.

alignment: AlignmentDirectional.topStart,
child: Container(
constraints: BoxConstraints(maxHeight: maxHeight, maxWidth: maxWidth),
child: Column(
crossAxisAlignment: CrossAxisAlignment.start,
mainAxisSize: MainAxisSize.min,
children: [
Padding(
padding: const EdgeInsetsDirectional.only(
start: 24,
top: 12,
end: 24,
),
child: Text(
GalleryLocalizations.of(context).demoOptionsTooltip,
style: textTheme.display1.apply(
color: colorScheme.onSurface,
fontSizeDelta:
isDisplayDesktop(context) ? desktopDisplay1FontDelta : 0,
),
),
),
),
Divider(
thickness: 1,
height: 16,
color: colorScheme.onSurface,
),
Flexible(
child: ListView(
shrinkWrap: true,
children: [
for (final configuration in configurations)
_DemoSectionOptionsItem(
title: configuration.title,
isSelected: configuration == configurations[configIndex],
onTap: () {
onConfigChanged(configurations.indexOf(configuration));
},
),
],
Divider(
thickness: 1,
height: 16,
color: colorScheme.onSurface,
),
),
SizedBox(height: 12),
],
Flexible(
child: ListView(
shrinkWrap: true,
children: [
for (final configuration in configurations)
_DemoSectionOptionsItem(
title: configuration.title,
isSelected: configuration == configurations[configIndex],
onTap: () {
onConfigChanged(configurations.indexOf(configuration));
},
),
],
),
),
SizedBox(height: 12),
],
),
),
);
}
Expand Down Expand Up @@ -594,11 +572,13 @@ class _DemoSectionInfo extends StatelessWidget {
const _DemoSectionInfo({
Key key,
this.maxHeight,
this.maxWidth,
this.title,
this.description,
}) : super(key: key);

final double maxHeight;
final double maxWidth;
final String title;
final String description;

Expand All @@ -607,33 +587,36 @@ class _DemoSectionInfo extends StatelessWidget {
final textTheme = Theme.of(context).textTheme;
final colorScheme = Theme.of(context).colorScheme;

return Container(
padding: const EdgeInsetsDirectional.only(
start: 24,
top: 12,
end: 24,
bottom: 32,
),
constraints: BoxConstraints(maxHeight: maxHeight),
child: SingleChildScrollView(
child: Column(
crossAxisAlignment: CrossAxisAlignment.start,
mainAxisSize: MainAxisSize.min,
children: [
Text(
title,
style: textTheme.display1.apply(
color: colorScheme.onSurface,
fontSizeDelta:
isDisplayDesktop(context) ? desktopDisplay1FontDelta : 0,
return Align(
alignment: AlignmentDirectional.topStart,
child: Container(
padding: const EdgeInsetsDirectional.only(
start: 24,
top: 12,
end: 24,
bottom: 32,
),
constraints: BoxConstraints(maxHeight: maxHeight, maxWidth: maxWidth),
child: SingleChildScrollView(
child: Column(
crossAxisAlignment: CrossAxisAlignment.start,
mainAxisSize: MainAxisSize.min,
children: [
Text(
title,
style: textTheme.display1.apply(
color: colorScheme.onSurface,
fontSizeDelta:
isDisplayDesktop(context) ? desktopDisplay1FontDelta : 0,
),
),
),
SizedBox(height: 12),
Text(
description,
style: textTheme.body1.apply(color: colorScheme.onSurface),
),
],
SizedBox(height: 12),
Text(
description,
style: textTheme.body1.apply(color: colorScheme.onSurface),
),
],
),
),
),
);
Expand Down