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

For example app make number of emoji columns dynamic based on screen width #134

Merged
merged 7 commits into from
Jun 7, 2023

Conversation

timmaffett
Copy link
Contributor

This change makes the number of columns for the emoji picker dynamic based on screen width. When running the example on windows I was surpised to see this ridiculous result:

emoji_picker_flutter_example 4_30_2023 12_40_14 PM

Here instead is a more expected behavior for an emoji picker (this is the example with this PR added):

emoji_picker_flutter_example 4_30_2023 12_41_41 PM

For clarity I split out all of the emoji size related calculations to the top of the build method to better illustrate to the users what is going on.

For mobile versions the number of columns remains a reasonable number (for Pixel3 emulator it remains 7 as it was hard coded, for more modern phones the number of emoji columns increases as one would expect).

Thanks for the nice package!

@Fintasys
Copy link
Owner

@timmaffett Thanks for your improvements to the example ! Code-wise it looks good to me, but I would like to avoid too much complexity in the basic example. I would prefer to create an extra example e.g. main-dynamic-columns.dart in the example folder. What do you think ?

@timmaffett
Copy link
Contributor Author

timmaffett commented Jun 1, 2023

@Fintasys I have moved the dynamic column sizing to it's own file.

@timmaffett
Copy link
Contributor Author

timmaffett commented Jun 4, 2023 via email

@Fintasys
Copy link
Owner

Fintasys commented Jun 5, 2023

Comment makes more sense. After that I gonna merge, thanks for the work!
Seems also Github actions are failing. Please run flutter format lib test example before pushing

@timmaffett
Copy link
Contributor Author

done and I ran dart format as well.

@Fintasys Fintasys merged commit 3618170 into Fintasys:master Jun 7, 2023
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