-
Notifications
You must be signed in to change notification settings - Fork 1
Feat/multiple projects support #293
base: production
Are you sure you want to change the base?
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/eddieantonio/predictive-text-studio/7gEECp9fUN72a11CSXrbX6D3CeAp |
@eddieantonio There are still work to do for this PR, but I noticed that some of the cypress test scenarios are failing because "the center of this element is hidden from view": I wasn't sure if others are having similar issues, and they suggest to add |
In general, adding a workaround like This kind of thing should be fixed, but it's probably not a big deal; it may be a symptom of greater problems, but that's unclear to me at this point. As long as there's a comment explaining why the workaround was made, and maybe a |
I should be able to start working on this PR next week as soon as I'm done with my finals 😞 . Any feedback or suggestions would be highly appreciated in the meantime! |
@yehee: you are not obligated to complete this PR. I will give feedback in case you want to finish it. |
src/app/pages/Customize.svelte
Outdated
<div class="customize__sidebar"> | ||
{#each projects as project} | ||
<div class="customize__sidebar--project" class:selected={project.id === id} on:click={() => id = project.id}> | ||
{(project.dictionaryName || "?").substring(0, 1)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tl;dr: .substring(0,1)
will break for many languages.
There's no good solution for this, but .substring(0,1)
is, unfortunately, not good to get the first "letter" of the dictionary name. You want the first extended grapheme cluster of the name, and there's no easy way to do that in vanilla JavaScript at the moment (aside from dragging in a dependency just for this one task).
Also, because JavaScript chooses a rather annoying way of representing Unicode, .substring(0, 1)
might actually return a broken character: an unpaired low surrogate. You can replicate this by getting the first "character" of the 💩 emoji:
> "💩".substring(0, 1)
'�'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found this thread on StackOverflow that might be an alternative to .substring(0,1)
: [...s][0]
which will give us the first "character" of s
assuming s
is not empty.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the ol' Array.from(s)[0]
trick works here to get the first code point. Unfortunately for us, the first code point is not necessarily the first graphem cluster, which Matthias Bynens points out in the accepted answer.
Try this:
> s = ("고추장").normalize("NFD")
'고추장'
> console.log(Array.from(s))[0])
Haha, again, this is a very tricky problem to solve in general, for all languages! 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohh that's very interesting... Thanks a lot I'll update the code accordingly!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason we're going with NFD
option for unicode normalization?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha, I would always choose NFC. Unless you think "ᄀ" is the correct first letter for "고추장" (I don't know Korean, I just like spicy food).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense! I was wondering about that :) Will make the change accordingly!
Motivation
Notable changes
{force: true}
option for Cypress testingFooter
componentBCP47Tag
componentaddManualEntryDictionaryToProject
,updateManualEntryDictionaryToProject
withputDirectEntry
project: number
parameter as the project ID of interest to related methodsLandingPage
logic:setLanguage
: callsputProjectData
that updates existing record if valid project ID is provided; creates a new record otherwise.putProjectData
returns the project ID which was created/modifiedsetLanguage
is calledUnknown Language
CustomisePage
logic:dictionaryName
attribute toRelevantKmpOptions
KMPJsonFileLanguage
,StoredProjectData
,StoredWordList
to@common/types
importProjectData
,exportProjectData
now imports/exports a list of project data (used to be a single record)files
now++id, name, wordlist, size, project
andKMPFileData
now++id, package, project
whereproject
is the foreign key toprojectData.id
8
PACKAGE_ID
Resolves #231, #188