search

Bestagons / quack

An app that increases the quality of life of Emory students by adding value to the experience of getting food at dining halls on campus.
https://quack-app.github.io/
MIT License
5 stars 2 forks source link

Homepage UI Updates #154

Closed oreorz closed 2 years ago

oreorz commented 2 years ago

New Homepage UI

UI overhaul for homepage, golden images were updated as a result. Screenshot shown below:

imageedit_1_2902276123

oreorz commented 2 years ago

Need some assistance with fixing the flutter tests. With the new UI, all tests requiring any scrolling fail and return a StateError "Bad state: Too many elements". I've tried a couple of things with no real success.

RafaelPiloto10 commented 2 years ago

@oreorz after some supppperrr long debugging and researching, I found your issue:

Since you are using a ListView for each tile and it is technically a Scrollable the Bad State: Too many elements error is coming from the fact that the test doesn't know which scrollable to use... the main one or the one for each station. Now, there is a potential flaw with this that I don't really care for now, but I wonder if we could miss any elements and not be able to scroll down to see them since you made the physics unscrollable.

In any case, the solution is to tell the scrollUntilVisible function which scrollable to use by doing the following:

final scrollable = find.byWidgetPredicate((widget) =>
          widget is Scrollable && widget.physics is ClampingScrollPhysics);

await tester.scrollUntilVisiible(Finder, delta, scrollable: scrollable);

You will need to update all tests that scroll to explicitly state which scrollable to use

oreorz commented 2 years ago

@oreorz after some supppperrr long debugging and researching, I found your issue:

Since you are using a ListView for each tile and it is technically a Scrollable the Bad State: Too many elements error is coming from the fact that the test doesn't know which scrollable to use... the main one or the one for each station. Now, there is a potential flaw with this that I don't really care for now, but I wonder if we could miss any elements and not be able to scroll down to see them since you made the physics unscrollable.

In any case, the solution is to tell the scrollUntilVisible function which scrollable to use by doing the following:

final scrollable = find.byWidgetPredicate((widget) =>
          widget is Scrollable && widget.physics is ClampingScrollPhysics);

await tester.scrollUntilVisiible(Finder, delta, scrollable: scrollable);

You will need to update all tests that scroll to explicitly state which scrollable to use

Oh wow, I just saw this after finding a solution of my own using dragUntilVisible and adding keys to widgets. Both seem to work very similarly but I think I'll just go with yours. To address your question, I'm not 100% sure about this but I think I remember also being worried about whether elements will be cut off but figured out that because I wrapped the ListView.builder in an Expanded widget, that takes care of that potential issue.

RafaelPiloto10 commented 2 years ago

@oreorz no worries. I also figured that, which is why I am not too bothered to extensively test. We'll find out when the scraper is integrated