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

Add Aircraft Tracker component implementation #447

Merged
merged 16 commits into from
Apr 13, 2023
Merged

Conversation

fdelcampo
Copy link
Contributor

No description provided.

Copy link
Contributor

@sebastian-aranda sebastian-aranda left a comment

Choose a reason for hiding this comment

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

I've left several comments, please take a look to them @fdelcampo. Thanks!

love/src/Utils.js Outdated Show resolved Hide resolved
Comment on lines 41 to 45
const data = getFlightTracker(state);
return {
status: data.status,
aircrafts: data.aircrafts,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: this could be:

const data = getFlightTracker(state);
return data;

And would yield the same result.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now try with return { ...data };

love/src/components/FlightTracker/FlightTracker.jsx Outdated Show resolved Hide resolved
love/src/components/FlightTracker/FlightTracker.jsx Outdated Show resolved Hide resolved
love/src/components/FlightTracker/FlightTracker.jsx Outdated Show resolved Hide resolved
love/src/components/FlightTracker/MapFlightTracker.jsx Outdated Show resolved Hide resolved
love/src/components/FlightTracker/MapFlightTracker.jsx Outdated Show resolved Hide resolved
love/src/components/FlightTracker/MapFlightTracker.jsx Outdated Show resolved Hide resolved
love/src/components/FlightTracker/MapFlightTracker.jsx Outdated Show resolved Hide resolved
love/src/components/icons/Zoom/ZoomInIcon.jsx Outdated Show resolved Hide resolved
Copy link
Contributor

@sebastian-aranda sebastian-aranda left a comment

Choose a reason for hiding this comment

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

I still suggest checking this comment: https://github.com/lsst-ts/LOVE-frontend/pull/447/files#r1164671409, although it is not urgent. I've also requested to change PR title name and also set your addition on the CHANGELOG to be the PR title @fdelcampo 🙏️

CHANGELOG.rst Outdated Show resolved Hide resolved
@fdelcampo fdelcampo changed the title Tickets/love 169 Aircraft Tracker component implementation Add Aircraft Tracker component implementation Apr 13, 2023
@fdelcampo fdelcampo merged commit e15b264 into develop Apr 13, 2023
@fdelcampo fdelcampo deleted the tickets/LOVE-169 branch April 13, 2023 15:04
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