Convert icon functionality from filename to number #108

liamboddin · 2023-08-30T18:21:32Z

…string for the database type

Server/src/models/api.ts

n1kPLV · 2023-09-01T19:32:47Z

Review-Hint: The Website/public directory contains static files, mostly icons.

n1kPLV

Looks good to me. However, some of these changes are my own, so I would like someone else to review this too

n1kPLV · 2023-09-02T13:13:43Z

Website/src/utils/api.ts

@@ -32,9 +32,20 @@ export type FullTrack = BareTrack & {

 export type TrackList = BareTrack[];

+export const POITypeIconValues = {


I used this instead of an enum for the website code, as the "backwards mapping" automatically added by TypeScript would have forced me to write messier code. This is functionally equivalent in most ways to an enum, when combined with the type definition below.

n1kPLV · 2023-09-02T13:14:28Z

Website/src/utils/common.ts

+};
+
+export const POIIconImg: Readonly<Record<POITypeIcon, string>> = {


I am still not quite sure if this file is the correct place for this definition, but I had to fit it somewhere...

n1kPLV · 2023-09-02T13:15:48Z

Website/src/utils/rotatingIcon.ts

As I have touched this file, it got reformatted to match the prettier rules defined for the website.

n1kPLV · 2023-09-02T13:20:11Z

Website/src/app/management/poi/client.tsx

@@ -186,14 +186,32 @@ export default function POIManagement({ poiTypes, tracks }: { poiTypes: POIType[
 								name={"selPoi"}
 								className="col-span-5 border border-gray-500 dark:bg-slate-700 rounded"
 								options={poiOptions}
+								unstyled={true}


To add "dark mode" support, it was easier to remove the default style from react-select and define my own style. This should look essentially the same in "light mode", and actually usable in "dark mode".

If this works, I will bring this style to the other react-select components too, possibly by extracting the style into a separate component

n1kPLV · 2023-09-02T13:23:24Z

Website/src/app/components/map.tsx

+	const enriched_poi_types: (POIType & { leaf_icon: L.Icon })[] = useMemo(
+		() =>
+			poi_types.map(pt => {
+				const icon_src = POIIconImg[pt.icon] ?? POIIconImg[POITypeIconValues.Generic];


The logic for creating the marker icon is now a little bit more complex than before. If the icon value is not one of the known ones, we will just use the "Generic" icon.

n1kPLV · 2023-09-02T13:28:59Z

Website/src/app/components/iconSelection.tsx

-	{ path: "/poiTypeIcons/lesser_level_crossing.svg", name: "Unbeschilderter Bahnübergang" },
-	{ path: "/poiTypeIcons/parking.svg", name: "Haltepunkt" }
-];
+const POI_ICONS: POITypeIcon[] = Object.values(POITypeIconValues);


This would have been much more messy, and much less type-safe with a typescript enum.

n1kPLV · 2023-09-02T13:31:08Z

Website/src/app/components/dynlist.tsx

-						<th className={"mx-2 border-b-black border-b px-2"}>Batterieladung</th>
-						<th className={"mx-2 border-b-black border-b px-2"}>Auf Karte anzeigen</th>
+						<th className={"mx-2 border-b-black dark:border-b-white border-b px-2"}>Name</th>
+						<th className={"mx-2 border-b-black dark:border-b-white hidden sm:table-cell border-b px-2"}>geog. Breite</th>


Optimization for mobile devices: Coordinates are hidden on small screens.

n1kPLV · 2023-09-02T13:32:46Z

Server/src/services/db/poi.controller.ts

@@ -71,6 +71,7 @@ export default class POIController {
 				},
 				data: {
 					name: name,
+					icon: icon,


I hope this doesn't collide too much with any changes made in #112

n1kPLV · 2023-09-02T13:36:41Z

Server/src/routes/init.route.ts

+				continue
+			}
+			// Check if the icon number is a member of the enum.
+			if (!(poiIcon in POITypeIcon)) {


As an enum is compiled down to an object, this should check -- using the backwards mapping -- if the number is a value assigned to an enum member.

liamboddin · 2023-09-03T08:09:02Z

It would be possible to remove some of the data sent over the API. I think it might be possible to remove the isTurningPoint-flag, change up the typeId in the POIs to something like icon. Right now the icon needs to be loaded with an extra request to POIType.

At this point I am not sure, if it would be clever to do that though, as this project is coming towards its end. The current version on this definitely runnable and works out fine. These changes would not change much in the performance as the POIs are rather considered static i.e. they are not requested very frequently

@n1kPLV what do you think about further API changes?

liamboddin · 2023-09-04T09:45:24Z

Although this would affect the App-API as well with the current API-Types... This change will not be a priority as of right now.

n1kPLV · 2023-09-04T09:50:59Z

I have already removed the option to set the isTurningPoint flag, always setting it to false. I too think that it is redundant, and I hope nothing relys on that flag being used correctly.

Converted icon to new functionality as a number for the API type but …

b1ac427

…string for the database type

liamboddin assigned n1kPLV and liamboddin Aug 30, 2023

liamboddin linked an issue Aug 30, 2023 that may be closed by this pull request

Change usage of icon in DB-POIType #107

Closed

liamboddin changed the title ~~Converted icon to new functionality as a number for the API type but …~~ Convert icon functionality from filename to number Aug 30, 2023

n1kPLV reviewed Sep 1, 2023

View reviewed changes

Server/src/models/api.ts Outdated Show resolved Hide resolved

n1kPLV added 4 commits September 1, 2023 11:22

Introduced POI Icon Enum

c8bd8ed

Made poi type icons updatable

f2ccc50

Frontend: Added POI Type icon support and updated POI icons

2271949

Decreased React warnings and improved dark mode

b1a0164

n1kPLV marked this pull request as ready for review September 1, 2023 19:31

n1kPLV reviewed Sep 2, 2023

View reviewed changes

n1kPLV added 2 commits September 2, 2023 16:09

Added image processing dependency.

ef49ac0

Removed isTurningPoint POI flag

fcf11f3

liamboddin linked an issue Sep 3, 2023 that may be closed by this pull request

Turning point POI type #106

Closed

liamboddin merged commit eea9be3 into development Sep 4, 2023
2 checks passed

liamboddin deleted the 107-poi-icon branch September 4, 2023 09:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert icon functionality from filename to number #108

Convert icon functionality from filename to number #108

liamboddin commented Aug 30, 2023

n1kPLV commented Sep 1, 2023

n1kPLV left a comment

n1kPLV Sep 2, 2023

n1kPLV Sep 2, 2023

n1kPLV Sep 2, 2023

n1kPLV Sep 2, 2023

n1kPLV Sep 2, 2023

n1kPLV Sep 2, 2023

n1kPLV Sep 2, 2023

n1kPLV Sep 2, 2023

n1kPLV Sep 2, 2023

liamboddin commented Sep 3, 2023 •

edited

Loading

liamboddin commented Sep 4, 2023 •

edited

Loading

n1kPLV commented Sep 4, 2023

		@@ -32,9 +32,20 @@ export type FullTrack = BareTrack & {

		export type TrackList = BareTrack[];

		export const POITypeIconValues = {

		};

		export const POIIconImg: Readonly<Record<POITypeIcon, string>> = {

Convert icon functionality from filename to number #108

Convert icon functionality from filename to number #108

Conversation

liamboddin commented Aug 30, 2023

n1kPLV commented Sep 1, 2023

n1kPLV left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liamboddin commented Sep 3, 2023 • edited Loading

liamboddin commented Sep 4, 2023 • edited Loading

n1kPLV commented Sep 4, 2023

liamboddin commented Sep 3, 2023 •

edited

Loading

liamboddin commented Sep 4, 2023 •

edited

Loading