buidlingreactUI #27

Closed
madeleinema wants to merge 2 commits from buidlingreactUI into master
Collaborator

Created frontend of a form to create events.

Created frontend of a form to create events.
madeleinema added 2 commits 2026-06-21 16:34:54 -04:00
dominic requested changes 2026-06-21 16:58:51 -04:00
@ -0,0 +24,4 @@
"react-router-dom": "^7.18.0",
"zod": "^4.4.3"
},
"devDependencies": {
Owner

why are dev dependencies different from prod dependencies?

why are dev dependencies different from prod dependencies?
@ -0,0 +1,184 @@
.counter {
Owner

is it react standard for the file to be uppercase App.css? having an uppercase letter in file is weird and i would only do it if it was required by react

is it react standard for the file to be uppercase App.css? having an uppercase letter in file is weird and i would only do it if it was required by react
@ -0,0 +22,4 @@
.base,
.framework,
.vite {
Owner

what actually is vite, why is it required

what actually is vite, why is it required
@ -0,0 +56,4 @@
}
}
#center {
Owner

are these better as ids or classes?

are these better as ids or classes?
@ -0,0 +104,4 @@
}
}
#next-steps ul {
Owner

i feel like a lot of these names are ambiguous. like next steps what, docs what? I imagine we will have a documentation page.

i feel like a lot of these names are ambiguous. like next steps what, docs what? I imagine we will have a documentation page.
@ -0,0 +115,4 @@
height: 18px;
}
a {
Owner

this implies we will have multiple link styles... i would take a look at a lot of your nested css and see if there are things that could be put at the global level, why do we need a special link here, it is just going to lead to an inconsistent UI

this implies we will have multiple link styles... i would take a look at a lot of your nested css and see if there are things that could be put at the global level, why do we need a special link here, it is just going to lead to an inconsistent UI
@ -0,0 +1,62 @@
import { MapContainer, TileLayer, Marker, Popup } from 'react-leaflet';
Owner

all imports should at least be in some sort of alphabetical order. idk how best to do that in react tho

all imports should at least be in some sort of alphabetical order. idk how best to do that in react tho
@ -0,0 +2,4 @@
import { useMapEvents } from 'react-leaflet';
import { Routes, Route, Link } from "react-router-dom";
import EventForm from './components/eventForm.jsx'; // <- add this import
Owner

what is a .jsx file anyway?

what is a .jsx file anyway?
@ -0,0 +3,4 @@
import { Routes, Route, Link } from "react-router-dom";
import EventForm from './components/eventForm.jsx'; // <- add this import
function ClickHandler() {
Owner

this function is just for debugging, probably should not be in master

this function is just for debugging, probably should not be in master
@ -0,0 +17,4 @@
}
// src/App.jsx
function Home() {
Owner

the lower function has export default function but this one just has function, why?

what do these functions even do?

the lower function has export default function but this one just has function, why? what do these functions even do?
madeleinema marked this conversation as resolved
@ -0,0 +29,4 @@
export default function App() {
return (
<div>
<Routes>
Owner

this has routes, the other doesn't, why?

this has routes, the other doesn't, why?
madeleinema marked this conversation as resolved
@ -0,0 +37,4 @@
);
}
// export default function App() {
Owner

delete commented code

delete commented code
madeleinema marked this conversation as resolved
@ -0,0 +1,124 @@
/* eventForm.css */
Owner

damn okay. we have a lot of css in multiple files. you need to probably really create a single css file and inherit it in most cases, with only minor tweaks for each page.

damn okay. we have a lot of css in multiple files. you need to probably really create a single css file and inherit it in most cases, with only minor tweaks for each page.
@ -0,0 +5,4 @@
- When a place is chosen, address + coordinates are written into the form values
- Coordinates are submitted as flat fields; the schema transforms them into `location`
*/
import React, {useEffect, useState} from "react";
Owner

organize imports better

organize imports better
@ -0,0 +29,4 @@
import markerRetinaUrl from "leaflet/dist/images/marker-icon-2x.png";
import markerShadowUrl from "leaflet/dist/images/marker-shadow.png";
L.Icon.Default.mergeOptions({
Owner

what does this do?

what does this do?
madeleinema marked this conversation as resolved
@ -0,0 +48,4 @@
watch,
} = useForm({
resolver: zodResolver(EventPostSchema),
defaultValues: {
Owner

this is good for dev, but we need to remove them for prod.

this is good for dev, but we need to remove them for prod.
@ -0,0 +61,4 @@
rain_date: "",
email: "madeleienma07@gmail.com",
phone_number: "8482189789",
latitude: "", // hidden field for latitude
Owner

remove ai slop comments

remove ai slop comments
@ -0,0 +78,4 @@
try {
// Schema handles transformation - data is already nested as { location: { latitude, longitude } }
await eventService.createEvent(data);
alert("Your event has been created successfully");
Owner

i dont love alerts for users. better to do message toast like django

i dont love alerts for users. better to do message toast like django
@ -0,0 +144,4 @@
// parse lat/lng from watched values
const markerPosition = (latitude && longitude) ? [parseFloat(latitude), parseFloat(longitude)] : null;
return (
Owner

why is this not in app.jsx then? im not saying to do that, but this is inconsistent.

why is this not in app.jsx then? im not saying to do that, but this is inconsistent.
@ -0,0 +167,4 @@
{/* Map and search control */}
<div style={{marginTop: 8}}>
Owner

you can probably abstract the map html into its own component, because it is going to have to be reused again

you can probably abstract the map html into its own component, because it is going to have to be reused again
@ -0,0 +185,4 @@
</div>
</div>
<div>
{/* Address input - if you want you can also update address automatically from search result */}
Owner

ai slop comment

ai slop comment
madeleinema marked this conversation as resolved
@ -0,0 +1,111 @@
:root {
Owner

damn, more css

damn, more css
@ -0,0 +30,4 @@
}
}
@media (prefers-color-scheme: dark) {
Owner

probably wana do prefers-color-scheme: system (idr if that is what it is tho)

probably wana do prefers-color-scheme: system (idr if that is what it is tho)
@ -0,0 +9,4 @@
export const EventPostSchema = z.object({
name: z.string().min(1, "Name is required"),
description: z.string().min(5, "Description has to be more than 5 letters."),
Owner

what is the first integer in min function mean.

what is the first integer in min function mean.
madeleinema marked this conversation as resolved
@ -0,0 +13,4 @@
url: z.string().url().nullable().optional(),
address: z.string().min(5, "Address is required"),
status: z.enum(["scheduled", "completed", "canceled"]).optional(),
Owner

seems like you have statuses written in two places, would be better if they were shared somehow

seems like you have statuses written in two places, would be better if they were shared somehow
@ -0,0 +21,4 @@
async createEvent(rawPayload) {
// Validate and coerce the outgoing payload
const parsed = EventPostSchema.parse(rawPayload); // will throw if invalid
console.log("EventService sending payload:", JSON.stringify(rawPayload, null, 2)); // Log here
Owner

ai slop comments and unnecessary console.log

ai slop comments and unnecessary console.log
@ -1,4 +1,8 @@
import sys
Owner

why is this file in here?

why is this file in here?
madeleinema closed this pull request 2026-06-21 23:48:55 -04:00

Pull request closed

Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: dominic/localist#27
No description provided.