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

allow variable height front pages #70

Closed
mockturtl opened this issue Jan 10, 2021 · 7 comments · Fixed by #73
Closed

allow variable height front pages #70

mockturtl opened this issue Jan 10, 2021 · 7 comments · Fixed by #73

Comments

@mockturtl
Copy link
Contributor

Is your feature request related to a problem? Please describe.

My first front page is a large form. It displays well.

Screenshot from 2021-01-10 17-34-12

My second front page is mostly empty:

Screenshot from 2021-01-10 17-33-08

I'd like to provide a target height for this page, so it expands to a more appropriate size for its content.

Screenshot from 2021-01-10 17-32-50

Describe the solution you'd like

I can get my desired behavior by adjusting the top param of the end rectangle in _getPanelAnimation.

Could we expose a variable for this?

Describe alternatives you've considered

I haven't explored how this will interact with the other BackdropScaffold constructor args.

Maybe PreferredSizeWidget is a useful way to convey the behavior?

Additional context

The Material spec doesn't address whether front pages should have a consistent height.

@daadu
Copy link
Member

daadu commented Jan 11, 2021

@mockturtl Have you tried stickyFrontLayer and headerHeight arguments of BackdropScaffold, to get desired UI?

@mockturtl
Copy link
Contributor Author

mockturtl commented Jan 11, 2021

  • Playing with values in example/lib/navigation.dart, my understanding of the stickyFrontLayer behavior is "When closed, the front layer will occupy as much height as possible without occluding the back layer. If the back layer occupies the entire screen height[1], this parameter has no effect."

[1] This is my use case: my back layer is a large menu. Apologies for misrepresenting that in the screenshot above.

  • headerHeight seems to control the height of the front layer in its inactive / closed state (this contradicts my reading of the docs[2]). Additionally, if stickyFrontLayer == true and headerHeight is less than the available space, headerHeight is ignored.

[2] It might be desirable to document the control flow inside the _headerHeight getter and _getPanelAnimation, if it's not too cumbersome to explain.


From what I can tell, the existing constructor args for BackdropScaffold don't allow me to specify "front page A opens to height X, front page B opens to height Y."

@willlarche Does Material have an opinion on a Backdrop with variable-height front layers? I'll double check Crane.

@daadu
Copy link
Member

daadu commented Jan 12, 2021

@mockturtl Thanks for pointing out problem in headerHeight docs, will correct it.

Regarding your requirement, as far as I understand(from checking Crane example), you want to have "extra" space under appbar, that is visible even when the backLayer is concealed.

A solution for that would be to set bottom of BackdropAppBar. Check the following code (modified example/main.dart):

MaterialApp(
      title: 'Backdrop Demo',
      home: BackdropScaffold(
        appBar: BackdropAppBar(
          title: Text("Backdrop Example"),
          actions: <Widget>[
            BackdropToggleButton(
              icon: AnimatedIcons.list_view,
            )
          ],
          bottom: PreferredSize(
            child: Column(
              children: [
                Text("Menu 1"),
                Text("Menu 2"),
              ],
            ),
            preferredSize: Size.fromHeight(100),
          ),
        ),
        backLayer: Center(
          child: Text("Back Layer"),
        ),
        subHeader: BackdropSubHeader(
          title: Text("Sub Header"),
        ),
        frontLayer: Center(
          child: Text("Front Layer"),
        ),
      ),
    );

Screenshot for it:
untitled

As far as I understand, the "extra" (always visible stuff) should be part of AppBar and not backLayer (which is revealed and concealed)

@willlarche
Copy link

@mockturtl Hi! So wonderful to see this implementation. Much thanks to everyone working on expanding Material coverage in the community.

We have no issue with that! Go for it!

@mockturtl
Copy link
Contributor Author

mockturtl commented Jan 12, 2021

Regarding your requirement, as far as I understand(from checking Crane example), you want to have "extra" space under appbar, that is visible even when the backLayer is concealed

Not really. I want the front layer's height to be configurable for each page. I'm not sure how to explain differently.

I'm happy to submit a PR if you think the idea makes sense.

Back layer and appbar look good

Screenshot from 2021-01-12 15-36-27

Front layer: Current behavior

Screenshot from 2021-01-12 15-36-32

Screenshot from 2021-01-12 15-31-34

Front layer: Desired behavior

Screenshot from 2021-01-12 15-47-03

@daadu
Copy link
Member

daadu commented Jan 13, 2021

@mockturtl Ok, got your point. Go ahead with the PR! BTW, what should we name the param? Any idea?

Few of my suggestions:

  • double frontLayerStartFrom: 0
  • double frontLayerTopMargin: 0
  • ??

Also while filing PR can you correct the docs for headerHeight - instead of using "closed" and "open" term (earlier we used them), use phrases like "when backLayer is concealed/revealed", that should avoid the confusion with the usage of `headerHeight.

mockturtl added a commit to mockturtl/backdrop that referenced this issue Jan 29, 2021
mockturtl added a commit to mockturtl/backdrop that referenced this issue Jan 29, 2021
@daadu daadu linked a pull request Feb 27, 2021 that will close this issue
@daadu
Copy link
Member

daadu commented Feb 27, 2021

This has landed with v0.5.0 on pub.dev.

@daadu daadu closed this as completed Feb 27, 2021
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 a pull request may close this issue.

3 participants