gaiety/actblue-tha
1
0
Fork 0
Code Issues Pull requests 1 Projects Releases Packages Wiki Activity Actions

implementation #1

Open
gaiety wants to merge 11 commits from implementation into main
pull from: implementation
merge into: gaiety:main
Conversation 0 Commits 11 Files changed 11 +910 -740
gaiety commented 2025-06-14 16:52:24 -06:00
Owner
Copy link

Take Home Assessment implementation

Comments available for what I'd do if I had more time :)

Take Home Assessment implementation Comments available for what I'd do if I had more time :)
gaiety added 11 commits 2025-06-14 16:52:24 -06:00
gaiety reviewed 2025-06-14 17:00:08 -06:00
package.json
@ -8,3 +8,2 @@
"@testing-library/user-event": "^13.5.0",
"react": "^18.2.0",
"react-dom": "^18.2.0",
"react": "^19.1.0",
gaiety commented 2025-06-14 16:53:02 -06:00
Author
Owner
Copy link

Latest React has nicer/simpler form action handling

Latest React has nicer/simpler form action handling
package.json
@ -35,1 +35,4 @@
]
},
"devDependencies": {
"html-react-parser": "^5.2.5",
gaiety commented 2025-06-14 16:55:17 -06:00
Author
Owner
Copy link

Description comes from JSON as raw HTML, this is a quick solution so we don't trust the HTML too far. But I'd do more research into the cleanest solution.

Extra points for the backend simply storing as markdown instead.

Description comes from JSON as raw HTML, this is a quick solution so we don't trust the HTML too far. But I'd do more research into the cleanest solution. Extra points for the backend simply storing as markdown instead.
package.json
@ -36,0 +36,4 @@
},
"devDependencies": {
"html-react-parser": "^5.2.5",
"react-imask": "^7.6.1"
gaiety commented 2025-06-14 16:54:32 -06:00
Author
Owner
Copy link

Many react mask plugins don't work, and very few address accessibility.

If this was a real project I'd pair with designers to find alternative ways to implement these forms without masking, more in line with the GOV.UK design system does.

Many react mask plugins don't work, and very few address accessibility. If this was a real project I'd pair with designers to find alternative ways to implement these forms _without_ masking, more in line with the GOV.UK design system does.
src/App.js
@ -7,4 +9,3 @@
function App() {
const bands = [skaBand, kpopBand, punkBand];
return (
<div className="App">
gaiety commented 2025-06-14 16:55:49 -06:00
Author
Owner
Copy link

Ideally this would be a page where you pick a band, but for now we just hard code [0].

Ideally this would be a page where you pick a band, but for now we just hard code `[0]`.
src/BandDetails.js
@ -0,0 +3,4 @@
function BandDetails({ band }) {
return (
<div className="details">
<img src={band.imgUrl} alt="" />
gaiety commented 2025-06-14 16:56:02 -06:00
Author
Owner
Copy link

Backend should return alt text for the images.

Backend should return alt text for the images.
src/BandDetails.js
@ -0,0 +4,4 @@
return (
<div className="details">
<img src={band.imgUrl} alt="" />
{parse(band.description_blurb)}
gaiety commented 2025-06-14 16:56:27 -06:00
Author
Owner
Copy link

Safely parse the HTML with html-react-parser, more research needed if this is the most ideal tool/method.

Safely parse the HTML with `html-react-parser`, more research needed if this is the most ideal tool/method.
src/BandForm.js
@ -11,0 +94,4 @@
<div className="total">
<h3>Total</h3>
<p aria-live="polite">${costToReadable(total)}</p>
gaiety commented 2025-06-14 16:57:31 -06:00
Author
Owner
Copy link

Need to test this with an actual screenreader. Should announce when prices change so the user is aware how much their "cart" costs.

Need to test this with an actual screenreader. Should announce when prices change so the user is aware how much their "cart" costs.
src/BandForm.js
@ -11,0 +99,4 @@
<div className="information">
<div className="form-row">
<input
gaiety commented 2025-06-14 16:57:51 -06:00
Author
Owner
Copy link

All of these should have autocomplete properties for easier autofill.

All of these should have `autocomplete` properties for easier autofill.
src/BandForm.js
@ -11,0 +127,4 @@
<fieldset className="payment">
<legend>Payment Details</legend>
{/* TODO: regex pattern */}
gaiety commented 2025-06-14 16:58:26 -06:00
Author
Owner
Copy link

Currently only checks that anything is entered, given more time I'd write regex for validation (likely against the HTML pattern attribute).

Currently only checks that anything is entered, given more time I'd write regex for validation (likely against the HTML `pattern` attribute).
src/BandHeader.js
@ -0,0 +3,4 @@
<header className="header">
<h1>{band.name}</h1>
<div className="header-details">
<time>{band.date}</time> {/* TODO: Format properly */}
gaiety commented 2025-06-14 16:59:16 -06:00
Author
Owner
Copy link

Needs pretty icon, to render not as an ugly number but as an elegent date.
Yet, should properly fill out the <time> attributes with what the computer friendly timestamp is.

Needs pretty icon, to render not as an ugly number but as an elegent date. Yet, should properly fill out the `<time>` attributes with what the computer friendly timestamp is.
src/BandHeader.js
@ -0,0 +4,4 @@
<h1>{band.name}</h1>
<div className="header-details">
<time>{band.date}</time> {/* TODO: Format properly */}
<address>{band.location}</address> {/* TODO: Format properly */}
gaiety commented 2025-06-14 16:59:26 -06:00
Author
Owner
Copy link

Needs icon, line break.

Needs icon, line break.
src/band-json/ska-band.json
@ -3,7 +3,7 @@
"id": "flaming-potatoes",
"date": 1683644012000,
"location": "Groove, 125 MacDougal St, New York, NY 10012",
"description_blurb": "<p>We're the Flaming Potatoes, and once you come to this awesome small club performance, you'll be our Best Spuds!</p>.",
gaiety commented 2025-06-14 16:59:44 -06:00
Author
Owner
Copy link

Rendered funny with a period after the paragraph? Likely a typo.

Rendered funny with a period _after_ the paragraph? Likely a typo.
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin implementation:implementation
git checkout implementation

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git checkout main
git merge --no-ff implementation
git checkout implementation
git rebase main
git checkout main
git merge --ff-only implementation
git checkout implementation
git rebase main
git checkout main
git merge --no-ff implementation
git checkout main
git merge --squash implementation
git checkout main
git merge --ff-only implementation
git checkout main
git merge implementation
git push origin main
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
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: gaiety/actblue-tha#1
No description provided.
Delete branch "implementation"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?