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

186719430 station selection list #22

Merged
merged 11 commits into from
Jan 26, 2024
Merged

Conversation

eireland
Copy link
Contributor

When user clicks on the selected station, a drop down of the 5 nearest stations to current location should be shown. User can then change the selected station.

@eireland eireland marked this pull request as draft January 24, 2024 23:28
@eireland eireland marked this pull request as ready for review January 25, 2024 00:27
Copy link
Contributor

@lublagg lublagg left a comment

Choose a reason for hiding this comment

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

Looks good - just one console.log to remove

const fromSecs = fromDate.getTime() / 1000;
const toSecs = toDate.getTime() / 1000;
const nearestStations: IStation[] = [];
console.log(adjustedStationDataset);
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove console.log?

src/utils/getWeatherStations.ts Outdated Show resolved Hide resolved
@eireland eireland requested a review from lublagg January 25, 2024 23:55
Copy link
Contributor

@lublagg lublagg left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Member

@dougmartin dougmartin left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@@ -33,8 +33,8 @@ export const App = () => {

useEffect(() => {
initializePlugin({pluginName: kPluginName, version: kVersion, dimensions: kInitialDimensions});
adjustStationDataset(weatherStations); //change max data to "present"
createStationsDataset(weatherStations); //send weather station data to CODAP
adjustStationDataset(weatherStations as IWeatherStation[]); //change max data to "present"
Copy link
Member

Choose a reason for hiding this comment

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

Its fine for this PR but it would be good in a future PR to convert the imported json file to a .ts file and type the data in the file so that if the types change but the imported data does not it is caught at compile time.

@lublagg lublagg merged commit 69d723a into main Jan 26, 2024
2 checks passed
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.

3 participants