Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix memory leaks with Plugin store layout #1510

Merged
merged 19 commits into from
Nov 10, 2022

Conversation

onesounds
Copy link
Contributor

@onesounds onesounds commented Nov 2, 2022

What's the PR

image

  • Decode Image Size in Theme Preview
    • Reason : if user use big size wallpaper, it will effect to flow. size decode will solve the problem.
    • I don't think it's a big problem, although the image drops to low definition.
  • Use Virtualizing Panels in Plugin Store List / plugin list / ETC for memory leak
  • Refactor Plugin Store UI (Changed Wrap panel from Uniform grid.)
    • Reason : in this case, item size doesn't fit with list width. Several implementations were attempted, but this condition was judged to be good because performance problems occurred and design problems were not significant.
  • 'Update Label' (Green Ball) changed placement to icon's right.
    • Reason : Due to implementation problems, it could not be displayed in the upper right corner.

Test Cases

  • No excessive memory leaks should occurrence with all menu.
  • Wallpaper displayed on the theme menu should be displayed in low resolution. (It has to be a level that doesn't bother you.)
  • Items in the plug-in store must be displayed appropriately even if you resize the window
  • The menu displayed when each item is clicked must operate normally.
  • Group (New, Installed....) and searches must operate normally.

@onesounds onesounds added Code Refactor kind/ui related to UI, icons, themes, etc labels Nov 8, 2022
@onesounds onesounds added this to the 1.10.0 milestone Nov 8, 2022
@onesounds onesounds marked this pull request as ready for review November 8, 2022 07:01
@onesounds
Copy link
Contributor Author

@taooceros 'refresh' button wrong work after merged Label PR , It seems like a simple task, but I couldn't fix it. Please add it on this pr or work on it separately. (Don't forget it!)

@jjw24
Copy link
Member

jjw24 commented Nov 8, 2022

@onesounds Are the test cases completed?

@onesounds
Copy link
Contributor Author

@onesounds Are the test cases completed?

in my case, Yes. but not sure. simply you can test (like when you did in smooth scroll pr) memory leak issue and working plugin store buttons. if its all fine, it is mergable.

@jjw24
Copy link
Member

jjw24 commented Nov 8, 2022

With dev branch, I can consistently push the memory usage up to 330mb on average by going to Settings -> Plugin Store scroll up and down a couple times -> exit and then repeat. Doing the same with this PR I see memory usage is around 250mb on average and only hits as high as 280mb rarely for a second but then quickly drops back down to the 250mb average.

🥇 @onesounds @taooceros

Copy link
Member

@jjw24 jjw24 left a comment

Choose a reason for hiding this comment

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

Installing plugins via mouse click is not working. Click on the install button then use mouse to click the install plugin result, nothing happens. Using enter works.

@jjw24 jjw24 added bug Something isn't working review in progress Indicates that a review is in progress for this PR labels Nov 8, 2022
@onesounds
Copy link
Contributor Author

Installing plugins via mouse click is not working. Click on the install button then use mouse to click the install plugin result, nothing happens. Using enter works.

ok. I'll check. when fix done, I'll mention you again. thanks.

@jjw24 jjw24 enabled auto-merge November 10, 2022 18:48
@jjw24 jjw24 removed the review in progress Indicates that a review is in progress for this PR label Nov 10, 2022
@jjw24 jjw24 merged commit 66cdc88 into Flow-Launcher:dev Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Code Refactor kind/ui related to UI, icons, themes, etc
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants