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

[AppBar] Hide on scroll #12337

Closed
2 tasks done
verekia opened this issue Jul 30, 2018 · 13 comments · Fixed by #15522
Closed
2 tasks done

[AppBar] Hide on scroll #12337

verekia opened this issue Jul 30, 2018 · 13 comments · Fixed by #15522
Labels
component: app bar This is the name of the generic UI component, not the React module! new feature New feature or request

Comments

@verekia
Copy link

verekia commented Jul 30, 2018

It is a common UX pattern to hide the AppBar when scrolling down on small screens.

There are different levels of implementation that are possible:

  • Minimal: Straight-up display: none the AppBar when scrolling down under the AppBar height. Even in that minimal setup, the case of the initial load when the user is already scrolled down has to be handled (I think always showing the bar at the initial load is the right UX). And it probably shouldn't auto-hide on large screen (unless an option is passed maybe).
  • With "headroom": Frequently when you scroll and release the screen or touchpad, the hand shakiness causes the scroll to go back a little bit, which makes the AppBar appear / disappear unexpectedly. Some headroom helps fixing this behavior.
  • Sliding: A smooth slide up / down transition makes it much more pleasant than display: none. Note: The AppBar shadow has to be taken into consideration, to not cause a shadow at the top of the page.
  • Multi-line AppBars: In the case where the AppBar has a second line of nav items, these should not get automatically hidden. I'm not sure if it's possible to even do that second line of items with Material UI, but it's worth noting here.
  • Android-like: When you scroll down from the top and from anywhere on the page, the AppBar stays where it is (like if it was in a simulated-position: static from the place you start scrolling down from, but it's actually still a position: fixed I think) and is only hidden by as much as you scroll. And when you scroll back up, the AppBar is revealed as much as you scroll up. In both cases, if you don't scroll up or down enough (less than half of the height of the AppBar), it snaps back to hiding or revealing itself and if you scroll more than half of the AppBar, it snaps to finishing the hiding / reveal completely.

I implemented it up to the Sliding part as a HOC to apply to AppBar but it's not as nice as having it natively supported by Material UI (demo of my implementation). Some Android examples.

The snapping feature is apparently a thing that got added in Android Library 23.1 (section Design).

  • This is a v1.x issue.
  • I have searched the issues of this repository and believe that this is not a duplicate.
@oliviertassinari oliviertassinari added new feature New feature or request component: app bar This is the name of the generic UI component, not the React module! labels Jul 31, 2018
@oliviertassinari
Copy link
Member

@verekia Thanks for sharing your work! This scrolling behavior is documented in the Official specification. It's something specific to small screens. I like the API you have been using, using a higher order component looks like a good tradeoff.
Looking at the implementation, it's not something I think that we should copy paste as it is. For instance, we should be able to remove recompose and lodash dependency.

I'm not sure if it's possible to even do that second line of items with Material UI, but it's worth noting here.

Yes, you can. It's just we don't provide any specific API for it yet.

@verekia
Copy link
Author

verekia commented Jul 31, 2018

Good call about the official doc, although they don't give much details. Yes, my HOC is really just a hopefully temporary crutch. It can be useful to give some basic guidance, but it's definitely not to take as-is.

@cvanem
Copy link
Contributor

cvanem commented Apr 17, 2019

@verekia @oliviertassinari
Do we still want to support this feature? What about something like this? https://codesandbox.io/s/7wok93mo20

I think this would provide the means to support all of the scrolling behavior outlined in the specification and it would be fairly easy to add to the AppBar component if we add a separate CollapseOnScroll hook. I do wonder if this is the best way to structure the functionality and if there are any performance concerns with handling the onScroll event? Some other libraries do throttling on the event handling.

@chr-knafl
Copy link

Hello!

I just started building my app with react and material ui and was searching for that feature, as I do believe is very important for the user experience. Vuetify supports this feature with a property on the component called "scroll-toolbar-off-screen" (see https://vuetifyjs.com/en/components/toolbars).
Besides that there are many more properties for configuring this behaviour (e.g. scroll-threshold, ...)

I really appreciate your efforts and wait for that feature to be included.

BR Christoph

@cvanem
Copy link
Contributor

cvanem commented Apr 26, 2019

@chr-knafl
I think the Material team must busy with the 4.0 release. Once that is done, I'm hoping we can look at getting this feature added. I am willing to work on a PR for this feature, but I think I would need a little guidance to be successful.

@chr-knafl
Copy link

@cvanem Thanks for your answer!
Your solution you added in the codesandbox seems to work properly. I did not test it on other browser than my chrome in the last version though. If there are some issues I will let you know!
BR Christoph

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 26, 2019

I agree that it's a useful feature and that a hook would be an excellent fit for the problem.

@cvanem Regarding the performance, you have an issue here. We need to reduce the number of re-rendering to the number of time the public API changes. Once we have a working hook, we might want to add a in like prop to the AppBar component to animate between the exit and enter state. Collapse doesn't provide the same animation. Maybe Slide

@chr-knafl Do you have a live example on Vuetify side? I can't see it working in your link.

Here is how I would approach the problem with v4, it's not perfect but a great start:

import React from "react";
import { Slide } from "@material-ui/core";

function getScrollY(scroller) {
  return scroller.pageYOffset !== undefined
    ? scroller.pageYOffset
    : scroller.scrollTop !== undefined
    ? scroller.scrollTop
    : (document.documentElement || document.body.parentNode || document.body)
        .scrollTop;
}

const useHideOnScroll = options => {
  const { threshold, scroller } = options;

  const scrollRef = React.useRef();
  const [hide, setHide] = React.useState(false);

  const handleScroll = React.useCallback(() => {
    const scrollY = getScrollY(scroller || window);
    const prevScrollY = scrollRef.current;
    scrollRef.current = scrollY;

    setHide(
      scrollY < prevScrollY
        ? false
        : scrollY > prevScrollY &&
          scrollY > (threshold != null ? threshold : 100)
        ? true
        : false
    );
  }, [scroller, threshold]);

  React.useEffect(() => {
    window.addEventListener("scroll", handleScroll);
    return () => {
      window.removeEventListener("scroll", handleScroll);
    };
  }, [handleScroll]);

  return hide;
};

export default function HideOnScroll(props) {
  const { children, threshold, scroller, ...other } = props;

  const hide = useHideOnScroll({ threshold, scroller });

  return (
    <Slide direction="down" appear={false} in={!hide} {...other}>
      {children}
    </Slide>
  );
}

https://codesandbox.io/s/5x8kq3nwjn (notice the addition of Box, CssBaseline and Container)

Apr-26-2019 23-07-20

@oliviertassinari oliviertassinari changed the title [AppBar] Auto-hide on scroll [AppBar] Hide on scroll Apr 26, 2019
@cvanem
Copy link
Contributor

cvanem commented Apr 28, 2019

@oliviertassinari Here is my first attempt at a real implementation: https://codesandbox.io/s/kknzvyq3p3?fontsize=14

I would appreciate it if someone could take a look and let me know if I am on the right track or should be headed in a different direction. Also, not sure if all of these changes would be allowed? I imagine they should be discussed before submitting a PR.

1. AppBar.js - I added the following props:

  • variant: Added 'default' and 'elevate' variants. The 'elevate' variable will elevate the app bar when the in prop is set.
  • elevation: Passes down the elevation to the child Paper component. If variant is set to 'elevate', and the in prop is set, then the elevation0 & elevationX classes are set to animate before passing down to the child Paper component.
  • in: Triggers the transition or elevation. The transition can also be triggered via TransitionProps
  • TransitionComponent: Copied this from Expansion panel.
  • TransitionProps: Copied this from Expansion panel.
  • transitionPosition - Depending on the transition and position settings, the transition behavior varies between the different transitions. The Slide transition works best while applied to the whole AppBar component, while Collapse works better when applied to the inner ToolBar component. This setting was added to allow configuration of the transition placement.

I was thinking we could possibly add a new Elevate transition component? This could replace the elevation logic in Paper.js and also be used here by passing in via TransitionComponent. If this is done, the new in and variant props would not be needed and can be removed. Just a thought, maybe there is a better way to accomplish this?

2. App Bar Expansion in Specification

I believe this implementation covers all of the App Bar scrolling behaviors outlined in the Official Specification except the expand example. I played around a bit with using the AppBar + ExpansionPanel. This approach works, but I am not sure if it is the correct approach as you are then limited to the expansion panel structure. Maybe the expansion implementation is for a later date...

3. demo.js - On Scroll logic and related hooks:

The hooks and scroll logic are pretty close to being finished. I think that the even't handlers may still need to be throttled as I am seeing some weird behavior sometimes on the Collapse with momentum scrolling. I ended up writing one hook for useTriggerOnScroll, which takes a triggerFunc parameter. This way, it can be used for both the elevation and hide triggers, which have a little different logic (see demo.js). Is this functionality something that could be added to the core package somewhere as a utility or should this be implemented by the user?

4. Example documentation

I added the above code as a separate example to the documentation, which appears to demonstrate the AppBar fairly well. It is pretty hard to show the user all the different settings, especially with static and fixed positions, and have it look good as a demo. Is there a better way to do the example documentation? Most AppBar's are typically set to 'fixed', but this doesn't show all that well in the example section. Maybe we should limit the user to only using pre-defined settings, or expose the pre-defined settings through different AppBar variants?

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 28, 2019

  1. AppBar.js - I added the following props:

@cvanem I think that we should avoid having the AppBar importing any transition component, nice call by not doing it. However, It seems that we only need the missing animation for the elevation. It should probably be added to the Paper component.

  1. App Bar Expansion in Specification
    I believe this implementation covers all of the App Bar scrolling behaviors outlined in the Official Specification except the expand example.

Do we need to handle it? It looks too complex and has a niche use case to be considered now. I would say no.

  1. demo.js - On Scroll logic and related hooks:

We shouldn't need a throttle function in the first place—I haven't looked at it in detail—it's the sign something else is off. I haven't considered the elevation case. Nice, here is an updated proposal: https://codesandbox.io/s/qx186qjqr9. I would probably only make the useAppBarScroll() hook public while adding demos for the Elevation component and HideOnScroll component. What do you think about it?

  1. Example documentation

We have an iframe option to render the demos in an iframe. It would be appropriate in our case.
I like the interactive demo idea. We only need to be careful, we shouldn't provide too many options people will be confused about.

@cvanem
Copy link
Contributor

cvanem commented Apr 29, 2019

@oliviertassinari

Thanks for the feedback. I like your simplified approach much better.

For the Scroll hook logic, a few things:

  1. I prefer a more generic hook that can be customized and open to more use cases. After reading, take a look at my new approach in useScrollTrigger.js. It uses the default trigger behavior from useAppBarScroll, but also allows customization. I think it is better as it addresses items a, b and c below, but I am fine if you want to use a more direct useAppBarScroll approach. Just curious, are there any other material-ui components that could benefit from a shared scroll trigger hook? I am considering publishing a react-use-scroll-trigger library as I haven't been able to find an implementation that does everything I want, which is:

    • a) Minimize hook update frequency and return a flag, aka trigger, only when a condition is met.
    • b) Implement a default trigger condition and also allow it to be customized.
    • c) Default the scroll target to window and also allow a ref to be passed in while correctly handling the ref state. More info here.
  2. Do we want the ability to specify a scroll target ref other than window? If so, I think we will need to provide a setRef callback instead of simply passing in the ref. Same reason/link as item c above

Here is my new approach, with your file structure: https://codesandbox.io/embed/7zjk7n51o0

See the changes in these files:

  • AppBar.js - Added PaperProps as I couldn't figure out a way to pass down the paper classes needed for the elevation transition.
  • ElevateOnScroll.js - Update to inject paper classes needed for the elevation transitions.
  • UseScrollTrigger.js - Proposed updated hook logic as described above (items a & b & c).
  • GrowScroll.js - Created to show an example of using an external ref (not using window).

@chr-knafl
Copy link

chr-knafl commented Apr 29, 2019

Do you have a live example on Vuetify side? I can't see it working in your link.

@oliviertassinari No, unfortunately not, I just read the official documentation of Vuetify. Maybe I can provide an example within the next weeks (I am currently quite busy - sorry).

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 29, 2019

@cvanem I love it! 😍🔥 Really nice. It's mature enough for opening a pull request. This way, we can discuss the changes in detail, line by line.

@fukemy
Copy link

fukemy commented Jul 19, 2024

Hi there, I am using ScrollToHide AppBar + Drawer, the problem I have got is when AppBar go hided, then I saw the space from Drawer to top, can anyone guide me to fix it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: app bar This is the name of the generic UI component, not the React module! new feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants