Word review redesign #379

tfnribeiro · 2024-05-21T06:46:12Z

Redesign of the Review Words page

Icons:

Introduced 5 new icons, one for the info boxes to make them slightly more distinct to incentivize reading them. The other 4 are two sets of + and - buttons to add and remove words.

Endpoints:

Added 2 endpoints to allow the frontend to update the bookmark fit for study flag: zeeguu/api#150 (changes in the backend)

Review Words Screen

The review screen has been changed to more easily allow the user to review the words which will be added to the exercises. Currently, the user could get almost all the words they translated to be available to be added to the exercises, which can be somewhat overwhelming for a begginer.

To get around this, we have decided that in case there are more than 10 words translated, we will prioritize the ones that are the most common in target language (L2), as these are likely to be the ones the user will encounter most when reading texts.

The user can eventually add more words from the word list by toggling the Manage Words For Exercises. I decided to include this inside the Infobox to further bring focus to it. To ensure this is understandable for users, we include a clickable text that will make the Tell me why modal appear that explains how words are selected for studying.

I have also disabled the possibility of adding translations longer than 3 words, as these will not fit will with the current exercise selection.

All of these changes mean that we have moved away from using the Star to "prefer" words to study. Instead, now the star is used to indicate that a user wants to study that word but it's not show in the UI anymore, and it's only tracked on the backend. This means that each word preview is now composed of the Edit + Listen button, which should make the UI nicer to look at.

Less than 10 words review:

More than 10 words review

Tell me why modal

Edit Words Screen

Finally, since we have also removed the trashcan, we have now moved this functionality to be part of the Edit button. Now the form includes a Delete Word button, which will remove the translation for the user. I also include a checkbox that reveals the state of whether the word is being considered for exercises or not: Include Word in Exercises.

+ Select words for the user to study based on their frequency counts in the learning language. This will help avoid situations where the user translates a lot of words in a single article and these would take up the entire pipeline. + Allow more user control by allowing them to edit words.

+ As words get deleted they need to notify their parent that the element shouldn't be rendered, otherwise there is a small space leftover. + Added a modal with an explanation to how words are selected.

+ Include Word in Exercises is now an editable field + Slight change to the delete button.

Considerations to prioritize words: 1. The user is reading an article and leaves without clicking on the review words -> We need to prioritize them when the component is unmounted. 2. The user goes to the review screen -> Do not perform it when the component is unmounted and only do it in the review screen. (Avoid the delay between API calls).

- Added explanation that the App remembers user altered words to prioritize studying.

+ Less emphasis on the delete button

+ Added constant for Max exercises per article.

Whenever a user selects a word to study, use the star functionality to remember that the user wants to study that word. This is done both in the Add/Minus and the Edit Form operations

Styling changes for Checkbox

Since the user may added bookmarks, update the number based on the wordsForExercises.

Added a title to word history

Remove console logging.

netlify · 2024-05-21T06:46:27Z

✅ Deploy Preview for voluble-nougat-015dd1 ready!

Name	Link
🔨 Latest commit	`8682bff`
🔍 Latest deploy log	https://app.netlify.com/sites/voluble-nougat-015dd1/deploys/66546d6d80d58200080607e2
😎 Deploy Preview	https://deploy-preview-379--voluble-nougat-015dd1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

mircealungu

overall cool stuff. a few questions for you in the comments.

mircealungu · 2024-05-27T07:27:40Z

src/exercises/ExerciseConstants.js

@@ -1,2 +1,3 @@
 export const MAX_EXERCISE_TO_DO_NOTIFICATION = 99;
 export const MAX_EXERCISE_IN_LEARNING_BOOKMARKS = 50;
+export const MAX_BOOKMARKS_PER_ARTILE = 10;


mircealungu · 2024-05-27T07:31:06Z

src/exercises/PrioritizeBookmarksToStudy.js

@@ -0,0 +1,36 @@
+import { MAX_BOOKMARKS_PER_ARTILE } from "./ExerciseConstants.js";


at least in my mind, file names should be uppercase if they define a component or a class. otherwise I was always using lowercase (see the /utils folder for example)

ah, but now i see that in the coding conventions this is not mandated:
https://github.com/zeeguu/web/blob/master/README.md

should we?

I hadn't noticed that was the reason why some files were lowercased, but now it makes sense!

We can do, but it would be good to add a line about it in the README then. I can do these changes.

mircealungu · 2024-05-27T07:32:08Z

src/exercises/PrioritizeBookmarksToStudy.js

@@ -0,0 +1,36 @@
+import { MAX_BOOKMARKS_PER_ARTILE } from "./ExerciseConstants.js";
+
+export default function prioritizeBookmarksToStudy(


also, does this belong to the exercises module, or is it more a dependency of the reader module? because i think it's called by the word review component in the reader. so maybe it should be placed there in that case?

I see your point. My argument is that the BookmarksToReview only exist because of exercises otherwise it would just be a list of words. In my view, I would expect to see Bookmarks related constants in the exercises and the Reader to only handle the logic of the aspects relating to the reading experience.

I can move it you prefer? But then what should it be called? We don't really have a file for these things for the Article.

mircealungu · 2024-05-27T08:20:27Z

src/reader/ArticleReader.js

@@ -345,7 +353,11 @@ export default function ArticleReader({ api, teacherArticleID }) {

      {readerReady && (
        <div id={"bottomRow"}>
-          <ReviewVocabulary articleID={articleID} />
+          <ReviewVocabulary


I know this is not your problem, but this confused me quite a bit because I though it was the review vocabulary page. Let's rename to ReviewVocabularyInvitationBox or ReviewWordsInfoBox or something to make it clear what's the nature of the component.

mircealungu · 2024-05-27T08:23:31Z

src/reader/ReviewVocabulary.js

-        </StyledButton>
-      </s.CenteredContent>
-    </s.FeedbackBox>
+      <s.FeedbackBox className="feedbackBox">


Again, I don't think this is your problem, but now I see it: why do we call this thing a FeedbackBox? There's no feedback provided in this panel. It's stupid, but even InteractiveBox would be better I think?

Moreover, the fact that we have a component, and then we have to also pass a "feedbackBox" class to it seems underwhelming!

Ditto, I will try to rename them to something more meaningful

mircealungu · 2024-05-27T08:24:41Z

src/reader/ToolbarButtons.js

-  },
-}));
+import { ThemeProvider } from "@mui/material/styles";
+import { t, Android12Switch } from "../components/MUIToggleThemes";


thanks for encapsulating this monster! :)

mircealungu · 2024-05-27T10:38:14Z

src/words/EditButton.js

@@ -39,10 +48,33 @@ export default function EditButton({
      bookmark.context,
    );
    api.updateBookmark(bookmark.id, newWord, newTranslation, newContext);
+    if (newFitForStudy) {
+      api.setIsFitForStudy(bookmark.id);


Given that we have eliminated the concept of "starring" maybe we should rename it completely and throughout the front-end and back-end to something more expressive, e.g. userAddedWordToStudyPossibilities or something like than that. Then the setFitForStudy should be done on the backend, because it's an internal detail of how we keep track of this user preference. Theoretically, in the future we might even decide to represent this state of a word w/o the help of the fitForStudy. Do you see what I mean?

I think with Starring I am just thinking if it ever meant something different in the past. If it's functionality was always to override to force it to exercises then, a userForceAddToExercises could be a good name for it, as that is what it also did in the past. However, it might be weird to load in the "star" from the bookmark table in the future.

Alright, I can rename the front_end methods to something more generic. Maybe a simple api.addToExercises and api.removeFromExercises and keep the logic in the backend.

mircealungu · 2024-05-27T10:40:37Z

src/words/Receptive.js

@@ -20,6 +20,11 @@ export default function Receptive({ api }) {
    setTitle(strings.titleReceptiveWords);
  }, [api]);

+  function deleteBookmark(bookmark) {


it took me a while to understand why this does not call the api. maybe if we named it notifyBookmarkDeleted it would be faster to grasp what it does?

What about removeDeletedBookmark, the method is not notifying any other component, just updating its' own list of the bookmarks displayed. Or maybe: updateOnDeletion

mircealungu · 2024-05-27T10:41:34Z

src/words/Word.js

 }) {
  const [starred, setStarred] = useState(bookmark.starred);
  const [deleted, setDeleted] = useState(false);
  const [reload, setReload] = useState(false);

+  function starBookmark(bookmark) {
+    api.starBookmark(bookmark.id);


or do we still have "starred" bookmarks after all?

I haven't removed the feature completely, as I didn't know if we might want to "revert" this at some point into a fully fledged field of it's own, where we just store what the user set (either in exercises or not). I think the semantics are still close enough to the original starring concept, that we could re-introduced starred words and not have many issues, but I could move away from it and just remove all code related with Deleting and Starring words (currently it's only commented out).

mircealungu · 2024-05-27T10:47:04Z

src/words/WordHistory.js

@@ -11,6 +11,16 @@ import { setTitle } from "../assorted/setTitle";
 export default function ReadingHistory({ api }) {
  const [wordsByDay, setWordsByDay] = useState(null);

+  function deleteBookmark(bookmark) {


ditto about notifyDelete

mircealungu · 2024-05-29T04:53:17Z

Ehi @tfnribeiro - I'm a bit lost in the state of the PRs. Please ping me explicitly when this needs to be reviewed again pls. Tak!

tfnribeiro · 2024-05-29T06:26:23Z

Ehi @tfnribeiro - I'm a bit lost in the state of the PRs. Please ping me explicitly when this needs to be reviewed again pls. Tak!

@mircealungu Maybe we should "close" this PR, as from the last conversation we considered using a new column in the back end to implement the user_preference (the star which would have 3 values: -1, 0, 1).

I will work on adding that field in backend and then re-open a PR aligned with that change?

mircealungu · 2024-05-29T07:38:32Z

Nice. Yes let's!

…

On Wed, 29 May 2024 at 09.26, Tiago Ribeiro ***@***.***> wrote: Ehi @tfnribeiro <https://github.com/tfnribeiro> - I'm a bit lost in the state of the PRs. Please ping me explicitly when this needs to be reviewed again pls. Tak! @mircealungu <https://github.com/mircealungu> Maybe we should "close" this PR, as from the last conversation we considered using a new column in the back end to implement the user_preference (the star which would have 3 values: -1, 0, 1). I will work on adding that field in backend and then re-open a PR aligned with that change? — Reply to this email directly, view it on GitHub <#379 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADRNBYLS2BT4O52CVZBXJ3ZEVYKJAVCNFSM6AAAAABIA7NQLOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMZWGYZTEMRQGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

tfnribeiro added 13 commits May 16, 2024 09:17

Notify Word Deletes + Modal Implementation

666808c

+ As words get deleted they need to notify their parent that the element shouldn't be rendered, otherwise there is a small space leftover. + Added a modal with an explanation to how words are selected.

Re-design Edit Form

a422f81

+ Include Word in Exercises is now an editable field + Slight change to the delete button.

Update ExplainBookmarkSelectionModal.js

fbe7d2e

- Added explanation that the App remembers user altered words to prioritize studying.

Rounded Edges on top message

8adc68e

Update FeedbackButtons.sc.js

80cc051

+ Less emphasis on the delete button

Update ExerciseConstants.js

7f049dc

+ Added constant for Max exercises per article.

Star to Remember

d67a0c5

Whenever a user selects a word to study, use the star functionality to remember that the user wants to study that word. This is done both in the Add/Minus and the Edit Form operations

Update WordEdit.sc.js

e0d2a6d

Styling changes for Checkbox

Update WordsToReview.js

20dffdb

Since the user may added bookmarks, update the number based on the wordsForExercises.

Update WordHistory.js

d6a074b

Added a title to word history

Update WordHistory.js

b8123ad

Remove console logging.

tfnribeiro added the enhancement New feature or request label May 21, 2024

tfnribeiro self-assigned this May 21, 2024

tfnribeiro linked an issue May 21, 2024 that may be closed by this pull request

More aggressively filter out words that get scheduled for practice #369

Closed

mircealungu self-requested a review May 24, 2024 08:22

mircealungu requested changes May 27, 2024

View reviewed changes

Merge branch 'master' into word-review-redesign

8682bff

mircealungu closed this May 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Word review redesign #379

Word review redesign #379

tfnribeiro commented May 21, 2024

netlify bot commented May 21, 2024 •

edited

Loading

mircealungu left a comment

mircealungu May 27, 2024

mircealungu May 27, 2024

tfnribeiro May 27, 2024

mircealungu May 27, 2024

tfnribeiro May 27, 2024

mircealungu May 27, 2024

mircealungu May 27, 2024

tfnribeiro May 27, 2024

mircealungu May 27, 2024

mircealungu May 27, 2024

tfnribeiro May 27, 2024

mircealungu May 27, 2024

tfnribeiro May 27, 2024 •

edited

Loading

mircealungu May 27, 2024

tfnribeiro May 27, 2024

mircealungu May 27, 2024

mircealungu commented May 29, 2024

tfnribeiro commented May 29, 2024

mircealungu commented May 29, 2024 via email

		@@ -0,0 +1,36 @@
		import { MAX_BOOKMARKS_PER_ARTILE } from "./ExerciseConstants.js";

		@@ -0,0 +1,36 @@
		import { MAX_BOOKMARKS_PER_ARTILE } from "./ExerciseConstants.js";

		export default function prioritizeBookmarksToStudy(

Word review redesign #379

Word review redesign #379

Conversation

tfnribeiro commented May 21, 2024

Redesign of the Review Words page

Icons:

Endpoints:

Review Words Screen

Less than 10 words review:

More than 10 words review

Tell me why modal

Edit Words Screen

netlify bot commented May 21, 2024 • edited Loading

✅ Deploy Preview for voluble-nougat-015dd1 ready!

mircealungu 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tfnribeiro May 27, 2024 • edited Loading

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

mircealungu commented May 29, 2024

tfnribeiro commented May 29, 2024

mircealungu commented May 29, 2024 via email

netlify bot commented May 21, 2024 •

edited

Loading

tfnribeiro May 27, 2024 •

edited

Loading