isNonScrolling() method in screen.presets doesn't behave correctly

[jvgeee]

I may be reading it completely wrong, but the current way the Screen component decides whether to use a scrollview or not is to check isNonScrolling(preset)

The code:

export function isNonScrolling(preset?: ScreenPresets) {
  // any of these things will make you scroll
  return !preset || !presets[preset] || preset === "fixed"
}

The comment "Any of these things will make you scroll" is incorrect - it implies that if you have no preset prop, it will make scroll. It should be the other way around.

If you're like me and want your pages to scroll by default, then you should just make this:

export function isNonScrolling(preset?: ScreenPresets) {
  return  preset === "fixed"
}

[robinheinze]

Hey @jvgeee! Thanks for the report!

In our case, we more commonly use fixed screens because a lot of the time, the screen will contain other scrolling elements, like FlatList, so we generally want the base screen to be fixed to avoid nested scrolls. But if scrolling is the more common pattern in your app, the beauty of Ignite is you can use it as a starting point and tweak the resulting app to meet your needs!

[jvgeee]

@robinheinze I think you misunderstand my point - I'm saying the comment at line 64 of app/components/screen/screen.presets.ts that says:

// any of these things will make you scroll

should be changed to

// any of these things will make the screen fixed

e.g. if there's no preset, or if the preset is fixed, your screen will be fixed. I'm not arguing with the choice to make the view fixed by default (maybe my other comment on how to make it scroll by default seemed that way). Just saying that the current description in the code could be clearer..

[robinheinze]

@jvgeee Oh! I'm so sorry, I did misunderstand! Thank you for clarifying! I'll re-open and hopefully get this fixed soon :)

[yulolimum]

With the removal of many inline comments for Ignite v8: Maverick, I removed this one as well since the name of the function implies what it does (the comment did indeed contradict that) :)