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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ritam #1

Merged
merged 33 commits into from Jun 1, 2021
Merged

Ritam #1

merged 33 commits into from Jun 1, 2021

Conversation

RitamChakraborty
Copy link
Contributor

@RitamChakraborty RitamChakraborty commented May 30, 2021

I managed to fix the problem the same way I did in my project and which I had already mentioned you. I still feel like there is a better way to do it. I'm thinking of posting it in StackOverflow hoping to find out a better approach.

Other than that ...

  • Refactored the whole project
  • Tried to maintain best practices as much as possible (Please don't tease me with null-safety, it still hunts me in my dreams 馃槥)
  • Separated the widgets as much as possible to keep the code cleaner
  • Also switched most of the business logic to a different service file (try to follow that in the future too, it's way easier to work that way). I could have used provider or another state management library to separate the view and service, but I'm very pleased to see you made the project without relying on third party libraries. I feel like plugins are creating bad habits us. We barely scratch the surface of what we can do just with the core flutter library.
  • Made the ui responsive to the size of the media devices, instead of using hard codes values.

Apart from that, I've few concerns too ...

  • The duration slider may not be working as intended. I'm not sure if such behavior is showing after I made the changes. Or I'm couldn't pick the duration change in the ui
  • It may happen that one or two bar changes after a shuffle in some of the sorting types.

If if you think the changes are good of accepting then do merge the PR. And also don't hesitate point out any mistakes I made. It'll be greatly appreciated.

@mahimagoyalx
Copy link
Owner

Hi Ritam, thank you for making a contribution to this project.

Highlights:
鉁擱efactored the code
鉁擱esponsive design
鉁擟olour picker button appearance
鉁擮ffset space
鉁擲topped sorting when shuffled

Concerns:
鉂孌uration slider not working (you already mentioned)
鉂孖nsertion sort not working
鉂孋olour picker button not working after some operation happens

Your pull request is highly appreciated, I will try to sort out the first two concerns but if you could please look upon the third.

@mahimagoyalx mahimagoyalx merged commit 9991e5f into mahimagoyalx:master Jun 1, 2021
@mahimagoyalx
Copy link
Owner

@RitamChakraborty Thank you for your worthwhile contribution :)

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.

None yet

2 participants