Skip to content

Conversation

@Cheong-Lau
Copy link
Contributor

Also some slight refactors for readability/minor performance.

@Cheong-Lau Cheong-Lau force-pushed the chore/clippy branch 2 times, most recently from 8846b22 to 3e4c969 Compare October 11, 2025 03:25
@mmstick mmstick requested review from a team October 12, 2025 19:31
Copy link
Member

@leviport leviport left a comment

Choose a reason for hiding this comment

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

I noticed a regression in the bluetooth and wifi applets after this change. The lists of bluetooth devices and wifi access points are empty when they shouldn't be.

Before:
bt-before
wifi-before

After:
bt-after
wifi-after

@Cheong-Lau
Copy link
Contributor Author

I noticed a regression in the bluetooth and wifi applets after this change. The lists of bluetooth devices and wifi access points are empty when they shouldn't be.

I didn't test those properly, my bad 😓, I don't have networkmanager on my system.

I realised that using Column::from_vec() and Row::from_vec() is a dumb idea most of the time, since it doesn't inspect the child elements to fit to their size (which is what broke this). Using with_children() like before works, but it reallocates an already allocated Vec for no real reason. Instead, when building a Vec<Element>, just push() directly into a Row or Column instead. It fits to the size properly and doesn't need to reallocate at the end. Couldn't do anything about the places where the len() of the Vec was needed, but oh well.

@Cheong-Lau Cheong-Lau requested a review from leviport October 14, 2025 09:21
@Cheong-Lau Cheong-Lau force-pushed the chore/clippy branch 2 times, most recently from 2551e4f to e5298f4 Compare October 14, 2025 11:46
leviport
leviport previously approved these changes Oct 14, 2025
Copy link
Member

@leviport leviport left a comment

Choose a reason for hiding this comment

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

No worries, thanks for the quick fix!

Upon retesting, the wifi and bluetooth lists are now populating as expected. I didn't notice any other issues with any of the other applets. I think this is good to go.

Also some slight refactors for readability/minor performance.
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