Skip to content

Conversation

@frozenhelium
Copy link
Contributor

No description provided.

@frozenhelium frozenhelium force-pushed the feature/completeness-vector-tile branch 3 times, most recently from f5ee940 to d407c4c Compare July 30, 2025 05:59
@frozenhelium frozenhelium requested review from ofr1tz and tnagorra July 30, 2025 06:12
@frozenhelium frozenhelium force-pushed the feature/completeness-vector-tile branch 2 times, most recently from 95d3f41 to 9a5f9cb Compare August 3, 2025 08:32
@frozenhelium frozenhelium marked this pull request as ready for review August 3, 2025 08:32
@frozenhelium frozenhelium force-pushed the feature/completeness-vector-tile branch from 9a5f9cb to 2f58bf5 Compare August 3, 2025 08:40
return (180 / Math.PI) * Math.atan(0.5 * (Math.exp(n) - Math.exp(-n)));
}

export interface GeoJsonProps {
Copy link
Contributor

Choose a reason for hiding this comment

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

We can be a bit more explicit about the name here.
E.g. TileMapServiceGeoJsonProps

reference?: number | undefined,
}

export function createGeoJsonFromTasks(
Copy link
Contributor

Choose a reason for hiding this comment

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

We can be a bit more explicit about the name here.

Comment on lines +65 to +67
const sorted = tasks.sort(
(a, b) => (a.taskId > b.taskId ? 1 : -1),
);
Copy link
Contributor

Choose a reason for hiding this comment

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

sorting tasks using sort() will most probably mutate the list

const usableHeight = window.innerHeight - 300;
const maxTileHeight = Math.floor(usableHeight / ROWS_PER_PAGE);
const tentetiveNumTiles = Math.max(
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const tentetiveNumTiles = Math.max(
const tentativeNumTiles = Math.max(

Copy link
Contributor

@tnagorra tnagorra left a comment

Choose a reason for hiding this comment

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

I have added some minor comments.

@frozenhelium
Copy link
Contributor Author

Closing in favor of #76

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