Skip to content

fix: use spacing parameter instead of custom spacing #3172

Merged
veloce merged 1 commit into
lichess-org:mainfrom
tom-anders:refactor/map-indexed-and-last
May 14, 2026
Merged

fix: use spacing parameter instead of custom spacing #3172
veloce merged 1 commit into
lichess-org:mainfrom
tom-anders:refactor/map-indexed-and-last

Conversation

@tom-anders

Copy link
Copy Markdown
Collaborator

Came across this by accident while changing other stuff :D

@veloce veloce left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure this is an improvement. Can you elaborate?

@tom-anders

Copy link
Copy Markdown
Collaborator Author

Hmm looking at it now on the next morning I think it's not worse,but also not really better :D

Looking a bit more at the rest of the code, maybe this whole function is not needed anyway, it effectively adds spacing between the widgets, which should also be achievable via the Rows's spacing parameter

@tom-anders tom-anders marked this pull request as draft May 13, 2026 09:34
@tom-anders tom-anders force-pushed the refactor/map-indexed-and-last branch from 1a1ece7 to 6a6e18c Compare May 13, 2026 19:07
@tom-anders

Copy link
Copy Markdown
Collaborator Author

Yep, the custom logic was unnecessary indeed, using spacing: 8 looks the same in the puzzle dashboard and the user perf cards. For the storm dashboard, there's a very slight difference, but it's actually an improvement: The boxes (all-time, this month, this week, today) are the exact same size now (it's very subtle, but I measured it in inkscape):

Before:

Screenshot_1778698879_before

After:

Screenshot_1778698906_after

@tom-anders tom-anders marked this pull request as ready for review May 13, 2026 19:09
@tom-anders tom-anders changed the title refactor: use mapIndexedAndLast instead of hand-rolled implementation fix: use spacing parameter instead of custom spacing May 13, 2026
@tom-anders tom-anders requested a review from veloce May 13, 2026 19:10

@veloce veloce left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! That logic came from a time when the Row spacing parameter was not yet a thing.

@veloce veloce merged commit d232eee into lichess-org:main May 14, 2026
1 check passed
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 this pull request may close these issues.

2 participants