Conversation
AnnieeBennie
commented
Mar 18, 2026
- changed the layout
- added more info buttons and explanation pop-ups for them
- adjusted colors to also work in a dark mode
- changed the wording of the infoboxes and infobox icon
✅ Deploy Preview for voluble-nougat-015dd1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Hi @mircealungu, I think I finished the refactoring of the Review Words page. I tested it on both iphone and android emulators, and it worked well. I also changed colors a bit. I think if we add those colors we talked about, we can also add some of them here (so I added that light purple for a button and Info icon). What do you think? |
mircealungu
left a comment
There was a problem hiding this comment.
a few change requests for you @AnnieeBennie
| }, | ||
| wontBeInExercises: { | ||
| mainInfo: "Phrases composed of 3 or more words are not used in the exercises.", | ||
| extraInfo: "You can still find them in Words or History tab.", |
There was a problem hiding this comment.
maybe make the Words a link?
| return ( | ||
| <s.Infobox> | ||
| <img src={APP_DOMAIN + "/static/icons/info-icon.png"} alt="" /> | ||
| <img src={infoIcon} alt="" /> |
There was a problem hiding this comment.
there is a reason for this:
- The browser extension -- When the web app runs inside the extension, window.location.origin is a chrome-extension:// URL, not zeeguu.org. So
static assets referenced as /static/icons/... wouldn't resolve. The fallback on line 17 catches this: if the origin doesn't match an expected
domain (zeeguu.org, netlify.app, localhost), it forces APP_DOMAIN to https://www.zeeguu.org, so image URLs like APP_DOMAIN +
"/static/icons/info-icon.png" point to the actual server.
how can we solve this?
| flex-direction: row; | ||
| align-items: center; | ||
| margin: 1em; | ||
| margin: 0.5em; |
There was a problem hiding this comment.
is this important? does 0.5em it make a difference in some situation
| {!exercisesEnabled ? ( | ||
| <Tooltip title="You need to translate words in the article first." arrow> | ||
| <span> | ||
| <StyledButton disabled style={{ backgroundColor: "#AAB1DE", color: "white" }}> |
There was a problem hiding this comment.
we need to make this dark-mode friendly
| </span> | ||
| </Tooltip> | ||
| ) : ( | ||
| <StyledButton navigation onClick={toExercises} style={{ backgroundColor: "#AAB1DE", color: "white" }}> |
| <WordsForArticleContainer> | ||
| <LeftContent> | ||
| <BackArrow /> | ||
| <WordsToReview |
There was a problem hiding this comment.
WordsToReview never uses the screenWidth. Maybe we can avoid passing it then?
| history.push(`/exercises/}`); | ||
| }; | ||
|
|
||
| function logGoingToExercisesAfterReview(e) { |
There was a problem hiding this comment.
this is never called. we should either call it or remove it.
| <WordsListColumn> | ||
| <h3 | ||
| style={{ | ||
| display: "flex", |
There was a problem hiding this comment.
should we define this as a separate style? or reuse an existing style?
| @@ -1,10 +1,10 @@ | |||
| import * as s from "./Infobox.sc"; | |||
| import { APP_DOMAIN } from "../appConstants"; | |||
| import infoIcon from "/static/icons/info-info-icon.png"; | |||
There was a problem hiding this comment.
Could we just use the MUI InfoOutlined icon since the project already imports from @mui/material. Something like that:
import InfoOutlinedIcon from "@mui/icons-material/InfoOutlined";
// in the styled-components file:
const StyledInfoIcon = styled(InfoOutlinedIcon)`
cursor: pointer;
flex-shrink: 0;
color: var(--text-secondary);
font-size: 22px;
`;
This eliminates both PNG files, both CSS variables (--more-info-icon), and works automatically in any theme. MUI is already a dependency so there's zero cost.
+ centered the infobox on mobile devices
|
@mircealungu I fixed acc to your comments, also I added dark mode styles for buttons |
+ now it shows the ZeeguuSelectedWords infobox correctly + it always shows the list of words that won't appear in exercises
|
@mircealungu I deleted those infoboxes, and now we have only two of them. One is shown always when the user translates some words, and one in case no words were translated but they somehow open this page |
|
@AnnieeBennie - what's the situation with this? are the changes we discussed yesterday implemented? |
|
@mircealungu yes, I changed the infoboxes we had. So now this one with a toggle shows up if a user translates at least 1 word. |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@AnnieeBennie I've played a bit more with the text on this page. Now it looks like this:
There is still something off with this though. For one, the Info icon, we don't really have such great info in there. The more important "info" is in the (?) popup rather. I wonder, if we could swap the two somehow. Have the (?) near "Manage words"... and have the Infobox be "How words are added to exercsies?" and then put the main explanation there. |
Moved spaced repetition explanation into info box, removed (?) from "Will appear in exercises" header, removed unused variables/imports, simplified redundant condition, removed debug console.log. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Ah! I was trying to see how it looks, and I committed by mistake to this branch. This is how it looks. What do you think @AnnieeBennie ?
|
|
I love the simplicity of the first one! And the explanation inside of the (?). We can surely "show the infobox on the first visit and then move it into the question icon". But that is a bit of extra code. Even the version with only the details on demand is great. And maybe the first header could be: "Will eventually appear in exercises". To make it clear that it's not straight away. (We could even put the (?) after eventually?) |
|
Yes, let’s do that! I’ll tag you when it’s done |
+ moved the infobox text into the moreInfoBox +created a toggleContainer instead of using infobox for the toggle
|
@mircealungu done. |
|
@AnnieeBennie - yes, it's strange in the middle. at hte end is best! |
|
The current solution is so much cleaner and nicer! Thanks @AnnieeBennie . I'll merge! |





