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

works on chrome and macos #73

Merged
merged 1 commit into from
Feb 25, 2023

Conversation

Jesse-Lucas1996
Copy link
Contributor

@Jesse-Lucas1996 Jesse-Lucas1996 commented Feb 19, 2023

Running on Windows 11:
image
image
imo the icons are a bit too large on windows?

we are getting a pixel overflow

Running on Chrome:
image
image

@Jesse-Lucas1996
Copy link
Contributor Author

Jesse-Lucas1996 commented Feb 19, 2023

The https://assets.pokemon.com/ is returning a CORS issue but the app does run now.
I also upgraded Flutter and all the packages

Also sorry for the super long PR

@felipecastrosales
Copy link

Uhhh, very good! 🚀
Probably resolved issue #37.

@Jesse-Lucas1996
Copy link
Contributor Author

@felipecastrosales I kept seeing these can I do it and no one did it, so I thought I just would

@0x1026
Copy link
Contributor

0x1026 commented Feb 24, 2023

I'm updating my work environments so I'm gonna take a while to check this PR.
At least, I debugged already with Chrome, the app needs some improvement for scrolling ^^

I saw some theme changes on the code.
@hungps, can you check this PR? It's interesting!

@felipecastrosales
Copy link

@hugovidafe I haven't tried the code yet, but likely the theme changes come from Flutter 3.7.0. Or not?

@0x1026
Copy link
Contributor

0x1026 commented Feb 25, 2023

@hugovidafe I haven't tried the code yet, but likely the theme changes come from Flutter 3.7.0. Or not?

Idk, I didn't follow the Flutter change logs. When I already have the updated environment, I will catch up with the news.

@hungps
Copy link
Owner

hungps commented Feb 25, 2023

@Jesse-Lucas1996
Thank you so much for your contribution!

@hugovidafe LGTM!
There are some minor things I would like to change, but we can do it later since this PR is quite large.
MacOS works great! the web version is runnable but we still need to switch to another image source in order to bypass the CORS, we can make another PR to do that. I don't have a window machine at the moment so I can't test it out.

If anyone can take some screenshots of the app running on the window, that would be great!
@Jesse-Lucas1996 Could you please add some screenshots of the app running on the web and desktop to the PR's description? Thanks!

@Jesse-Lucas1996
Copy link
Contributor Author

@hungps I sure can do that for you :) if you have discord or something or even talk here it'll be great to figure out that image source as well

@Jesse-Lucas1996
Copy link
Contributor Author

@hugovidafe
Heyo! The theme changes do come from flutter being upgraded. This is the official change they recommended. It looks the same to me but don't take that at face value 😂

@hungps
Copy link
Owner

hungps commented Feb 25, 2023

@Jesse-Lucas1996
I've mentioned about how to use the new image source in this issue.
I still need to replace all imageurl from the data source with the new URL and bring the data source to flutter_pokedex_db so that everyone will be able to contribute.
Let's do all that on another PR 😉

@Jesse-Lucas1996
Copy link
Contributor Author

Jesse-Lucas1996 commented Feb 25, 2023

@hungps have updated with pictures
We might need to do a styling PR as well imo

@hungps
Copy link
Owner

hungps commented Feb 25, 2023

@Jesse-Lucas1996 Agree! Responsive is required. We could limit the width of the app for now so that the layout won't break.
flutter_web_frame looks promising 🤔

@Jesse-Lucas1996
Copy link
Contributor Author

@hungps I'm all or installing random things and pray that it works, should we merge this PR in then try that?

@hungps hungps merged commit 95e721f into hungps:master Feb 25, 2023
@hungps
Copy link
Owner

hungps commented Feb 25, 2023

@Jesse-Lucas1996 Merged!

@Jesse-Lucas1996 Jesse-Lucas1996 deleted the jesse/web-desktop-support branch February 25, 2023 10:41
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

4 participants