lichess-org / mobile

Lichess mobile app v2
GNU General Public License v3.0
1.37k stars 198 forks source link

refactor: add common widget for BottomBar #938

Closed tom-anders closed 2 months ago

tom-anders commented 3 months ago

This gets rid of a lot of code duplication and makes it more straight-forward to build new screens for new features.

Note that the Row in BottomBar now has MainAxisAlignment.spaceAround by default, so it's not necessary anymore to wrap buttons in Expanded.

veloce commented 3 months ago

While I think it is a good change, we must be careful it doesn't break some screens. We should make a list of all impacted screens and check they still look fine.

Space around is the same as Expanded if all the children are wrapped in an Expanded. This may be not the case everywhere (and is why I used Expanded instead of spaceAround I believe), as you might want to expand some buttons only.

tom-anders commented 3 months ago

While I think it is a good change, we must be careful it doesn't break some screens. We should make a list of all impacted screens and check they still look fine.

I'll add Screenshots for all impacted screens. 👍

Space around is the same as Expanded if all the children are wrapped in an Expanded. This may be not the case everywhere (and is why I used Expanded instead of spaceAround I believe), as you might want to expand some buttons only.

Yeah that's why I exposed the mainAxisAlignment property, so expanding only one of the buttons should still be possible. However, all the current bottoms bars seems to either use spaceAround alignment or Expanded for all buttons. I'll double check once again though.

tom-anders commented 2 months ago

Analysis:

Screenshot_1725200820

Opening Explorer:

Screenshot_1725200994

Board Editor:

Screenshot_1725200889

Offline Correspondence:

Screenshot_1725200974

Archived Game:

Screenshot_1725201013

Game Loading:

Screenshot_1725201066

Online Game:

Screenshot_1725201122

Puzzle Screen:

Screenshot_1725201175

Puzzle Storm:

Screenshot_1725201191

Puzzle Streak:

Screenshot_1725201210

TV:

Screenshot_1725201308

veloce commented 2 months ago

Thanks for the screenshots @tom-anders !