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 hall of fame images #105

Merged
merged 6 commits into from
Jul 8, 2023
Merged

Fix hall of fame images #105

merged 6 commits into from
Jul 8, 2023

Conversation

Ultimatum22
Copy link
Contributor

I kind of rewrote the logic to be more generic and account for changes on formula1 side, as long as they don't change the html.

I was a bit confused about the endpoint check because the one from Hive is never the same as the one set hardcoded in formula_one.dart, because the default values differ. I left it the same for now.

@BrightDV BrightDV linked an issue Jul 8, 2023 that may be closed by this pull request
@BrightDV
Copy link
Owner

BrightDV commented Jul 8, 2023

Thanks for the PR! The code is a lot more cleaner that what I have done... (I'm not proud of it though)
Also, can you add a screenshot of the working list?

I was a bit confused about the endpoint check because the one from Hive is never the same as the one set hardcoded in formula_one.dart, because the default values differ. I left it the same for now.

I know that it is a bit confusing, but I created the server to support the web version (mostly). So, when using a boxbox server, the API url has to be changed, but also the main website url, as it is blocked by the CORS too. So when the server is changed (e.g., the endpoint here), the main website url is changed too.
I hope that it is clear enough.

String driverName = driverInfo[0];
String driverYears = driverInfo[1];

dom.Element imageElement = element.getElementsByTagName('img').firstWhere((e) => e.classes.contains('hidden'));
Copy link
Owner

Choose a reason for hiding this comment

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

Can you add a trailing comma and then run a dart format . --set-exit-if-changed to format the code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't get the trailing icon here. I did applied the formatter, I was thinking of adding it to a workflow to fix this on the fly.

Copy link
Owner

Choose a reason for hiding this comment

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

I don't get the trailing icon here. I did applied the formatter, I was thinking of adding it to a workflow to fix this on the fly.

Yes, no worries. I will do a proper workflow for the PRs with formatting and analysing .

@Ultimatum22
Copy link
Contributor Author

Screenshot_20230708_150748.jpg

Ah damn, sorry forgot about the screenshot.

@Ultimatum22
Copy link
Contributor Author

Thanks for the PR! The code is a lot more cleaner that what I have done... (I'm not proud of it though) Also, can you add a screenshot of the working list?

I was a bit confused about the endpoint check because the one from Hive is never the same as the one set hardcoded in formula_one.dart, because the default values differ. I left it the same for now.

I know that it is a bit confusing, but I created the server to support the web version (mostly). So, when using a boxbox server, the API url has to be changed, but also the main website url, as it is blocked by the CORS too. So when the server is changed (e.g., the endpoint here), the main website url is changed too. I hope that it is clear enough.

Ah that does make sense, only the endpoint variable set now is never used so always different.

I can refactor the rest of the class if you want me too.

@BrightDV
Copy link
Owner

BrightDV commented Jul 8, 2023

Ah that does make sense, only the endpoint variable set now is never used so always different.

I can refactor the rest of the class if you want me too.

You can if you want to, but I think that a rewrite would be more appropriate when a function breaks, like this one. I'm not sure if it is a really good way to do, but at least it is easier.
Anyway, thanks for the PR & your efforts.

@BrightDV BrightDV merged commit 1f79f3b into BrightDV:main Jul 8, 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.

[BUG] Hall of Fame images not loading
2 participants