Conversation
|
The locmax issue has been fixed in PR #244. |
jesscmoore
left a comment
There was a problem hiding this comment.
HI @tonypioneer Thanks for actioning so quickly. Looks good. Can you make these SolidScaffoldHelpers.isXScreen methods exported?
Also, the SolidScaffoldHelpers now looks to replace WindowSize class. Please remove the duplication. This will now be a breaking change, as some apps use WindowSize.
Hi @jesscmoore, the SolidScaffoldHelpers.isXScreen methods have been exported. I’ve reviewed both SolidScaffoldHelpers and WindowSize, but I couldn’t find a clear duplication in the current implementation. They seem to overlap conceptually, but the structure and usage are still a bit different. Would you mind pointing me to the specific parts where you see duplication? That would really help me align the changes properly and avoid removing anything that’s still in use. |
jesscmoore
left a comment
There was a problem hiding this comment.
Hi @tonypioneer,
I've reviewed optimisation advice as well.
-
MediaQuery.of - is generally not advisable for performance reasons as it will trigger a full rebuild of everything affected when any MediaQuery property changes. MediaQuery.sizeOf() is better as will only rerun if screen size changes.
-
Using LayoutBuilder constraints, such as in WindowSize() is usually what one actually wants for responsive design, as it returns the size of the available layout, and only changes if the parent widget changes and is thought to have higher performance. I would suggest adapting the SolidScaffoldHelpers to use LayoutBuilder.
-
WindowSize.isNarrowWindow() is an example using constraints from LayoutBuilder.
-
We would be better to just have one type of method for wide measurement, or clarify the difference if there is uses for both. For notepod, communitypod usecases the need is to measure the layout width not the screen width.
Pull Request Details
What issue does this PR address
Associated Issue
Type of Change
How Has This Been Tested?
Build and run the app.
Checklist
Complete the check-list below to ensure your branch is ready for PR.
make preporflutter analyze lib)dart testoutput or screenshot included in issue #Finalising
Once PR discussion is complete and reviewers have approved: