SG's Little Fixes #242
SG's Little Fixes #242
Conversation
unfortunately, we need to add this to the `<body>` tag because modals (and perhaps script menus eventually) get moved outside of the main div by headless ui, so they don't take on the dark mode classes without this being at a higher level. Tailwind recommends this being at the body or html tag level, so I think this little dom manipulation is ok.
this is mentioned in GTCMT/earsketch#2260
| } else { | ||
| document.body.classList.remove(("dark")) | ||
| } | ||
| }, [theme]) |
There was a problem hiding this comment.
Okay, I see why this is necessary.
At some point, we might want to add a bit of JS to do this in public/index.html (though I think this is moot for now because our tailwind CSS isn't loaded yet), and perhaps respect the OS/browser theme setting by default.
(See https://tailwindcss.com/docs/dark-mode#toggling-dark-mode-manually)
scripts/src/app/SoundUploader.tsx
Outdated
| <input type="radio" style={{ marginRight: "0.75rem" }} checked={index === selected} | ||
| <div className="overflow-y-auto border px-3 border-gray-300 dark:border-gray-500" style={{ maxHeight: "300px" }}> | ||
| {results.map((result, index) => <div key={index} className={index === selected ? "bg-blue-200 dark:bg-blue-900" : ""}> | ||
| <div className="grid grid-flow-col items-center pt-3"> |
scripts/src/app/SoundUploader.tsx
Outdated
| {results.map((result, index) => <div key={index}> | ||
| <label> | ||
| <input type="radio" style={{ marginRight: "0.75rem" }} checked={index === selected} | ||
| <div className="overflow-y-auto border px-3 border-gray-300 dark:border-gray-500" style={{ maxHeight: "300px" }}> |
There was a problem hiding this comment.
Nice job on the UI here. Two requests:
- It'd be nice if there were a little left padding for the radio button. You could just move the padding from this element (
px-3) down to the children, so that the rows go all the way across. - This looks fine in Chrome, but there's a lot of extra vertical space in Firefox:
There was a problem hiding this comment.
ok, this is fixed, and back as children of the <label>
scripts/src/app/SoundUploader.tsx
Outdated
| <div className="overflow-y-auto border px-3 border-gray-300 dark:border-gray-500" style={{ maxHeight: "300px" }}> | ||
| {results.map((result, index) => <div key={index} className={index === selected ? "bg-blue-200 dark:bg-blue-900" : ""}> | ||
| <div className="grid grid-flow-col items-center pt-3"> | ||
| <input id={"fs" + index} type="radio" style={{ marginRight: "0.75rem" }} checked={index === selected} |
There was a problem hiding this comment.
I see that the dynamic IDs here are for the benefit of the <label>s.
Could we make these elements children of the <label> and scrap the IDs, or does that mess up the layout?
There was a problem hiding this comment.
Could probably be done with enough non-tailwind css. Would need to push the label back out to the right, because this would put it on the left. Not sure we could use grid either, as these would be children instead of siblings.
I don't think it's worth it.
scripts/src/user/Notifications.tsx
Outdated
| failure2: "#d73636", | ||
| success: "#065f46", | ||
| failure1: "#b91c1c", | ||
| failure2: "#b91c1c", |
There was a problem hiding this comment.
This is good; per our discussion, I'm guessing this will either grow to include background colors or get moved to its own PR.
There was a problem hiding this comment.
oh, and let's replace failure1 and failure2 with just failure, and get rid of fallback.
darkclass back up to<body>tag to capture modals which get moved outside of the main<div>by headlessCurrent layout
New Layout