Skip to content

Multiple Reversals completed implementation #807

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Multiple Reversals completed implementation #807

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions src/lib/components/LexiconLanguageTabs.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import { expoInOut } from 'svelte/easing';
import { fly } from 'svelte/transition';

export let reversalLanguage;
export let reversalLanguages = [];
export let selectedLanguage;
export let onSwitchLanguage;
export let vernacularLanguage;
Expand All @@ -25,21 +25,24 @@
></div>
{/if}
</div>

{#each reversalLanguages as lang}
<div
role="button"
tabindex="0"
aria-pressed={selectedLanguage === reversalLanguage}
on:click={() => onSwitchLanguage(reversalLanguage)}
on:keydown={(e) => e.key === 'Enter' && onSwitchLanguage(reversalLanguage)}
aria-pressed={selectedLanguage === lang}
on:click={() => onSwitchLanguage(lang)}
on:keydown={(e) => e.key === 'Enter' && onSwitchLanguage(lang)}
class="py-2.5 px-3.5 text-sm uppercase text-center relative dy-tabs dy-tabs-bordered mb-1"
>
{reversalLanguage}
{#if selectedLanguage === reversalLanguage}
{lang}
{#if selectedLanguage === lang}
<div
transition:fly={{ axis: 'x', easing: expoInOut, x: -70 }}
class="absolute -bottom-1 left-0 w-full h-1 bg-black"
></div>
{/if}
</div>
{/each}
<div class="flex-1"></div>
</div>
4 changes: 2 additions & 2 deletions src/lib/components/LexiconListViewHeader.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
export let alphabet = [];
export let selectedLanguage;
export let vernacularLanguage;
export let reversalLanguage;
export let reversalLanguages = [];
export let selectedLetter;
export let onSwitchLanguage;
export let onLetterChange;
Expand All @@ -26,7 +26,7 @@
</script>

<LexiconLanguageTabs
{reversalLanguage}
{reversalLanguages}
{selectedLanguage}
{onSwitchLanguage}
{vernacularLanguage}
Expand Down
112 changes: 57 additions & 55 deletions src/routes/lexicon/+page.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,10 @@
let selectedWord = null;
let defaultReversalKey = Object.keys(reversalAlphabets[0])[0];
let loadedReversalLetters = new Set();
let reversalWordsList;
let vernacularWordsList;
let reversalWordsList = [];
let vernacularWordsList = [];
let selectedLanguage = vernacularLanguage;
const reversalLanguageList = Object.values(reversalLanguages[0]);
let scrollContainer;
Comment on lines 38 to 44
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

defaultReversalKey & reversalLanguageList assume a single language

Both variables are derived from the first element of their arrays, causing incorrect fetch paths and “language not found” behaviour when the user picks another reversal language. You can compute them on demand:

-const defaultReversalKey = Object.keys(reversalAlphabets[0])[0];
-const reversalLanguageList = Object.values(reversalLanguages[0]);
+const reversalAlphabetMap = Object.fromEntries(
+    reversalAlphabets.flatMap((obj) => Object.entries(obj))
+);
+const reversalLanguageMap = Object.fromEntries(
+    reversalLanguages.flatMap((obj) => Object.entries(obj))
+);
+const getKeyByLang = (lang) =>
+    Object.keys(reversalLanguageMap).find((k) => reversalLanguageMap[k] === lang);

This map can then drive loadLetterData, handleLetterChange, scrolling logic, etc.

Committable suggestion skipped: line range outside the PR's diff.


// Subscribe to stores
Expand All @@ -52,21 +53,15 @@
const reversalLanguage = Object.values(reversalLanguages[0])[0];

async function fetchWords(letter = selectedLetter) {
if (selectedLanguage === reversalLanguage && !loadedReversalLetters.has(letter)) {
console.log('Loading letter data:', letter);

if (selectedLanguage !== vernacularLanguage && !loadedReversalLetters.has(letter)) {
const letterIndex = alphabets.reversal.indexOf(letter);
const lettersToLoad = alphabets.reversal
.slice(0, letterIndex)
.filter((l) => !loadedReversalLetters.has(l));

// Load all required letters in parallel
await Promise.all(lettersToLoad.map(loadLetterData));

// Load the current letter
await loadLetterData(letter);

// Sort the results based on the selectedLanguage's alphabet
reversalWordsStore.update((words) => {
const updatedWords = { ...words };
updatedWords[selectedLanguage] = (updatedWords[selectedLanguage] || []).sort(
Expand All @@ -82,53 +77,57 @@
});
}
}

async function loadLetterData(letter) {
async function loadLetterData(letter) {
let fileIndex = 1;
let moreFiles = true;
let newWords = [];

const index = reversalIndexes[defaultReversalKey];
const files = index[letter] || [];
for (const file of files) {
const reversalFile = `${base}/reversal/${defaultReversalKey}/${file}`;
const response = await fetch(reversalFile);
if (response.ok) {
const data = await response.json();
const currentFileWords = Object.entries(data).map(([word, entries]) => {
return {
word: word,
indexes: entries.map((entry) => entry.index),
vernacularWords: entries
.map((entry) => {
const foundWord = vernacularWordsList.find(
(vw) => vw.id === entry.index
);
if (foundWord) {
return {
name: foundWord.name,
homonymIndex: foundWord.homonym_index || 0
};
} else {
console.log(
`Index ${entry.index} not found in vernacularWordsList`
while (moreFiles) {
try {
const response = await fetch(
`${base}/reversal/${defaultReversalKey}/${letter}-${String(fileIndex).padStart(3, '0')}.json`
);
if (response.ok) {
Comment on lines +81 to +90
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Letter data is always fetched from the first reversal language

loadLetterData hard-codes defaultReversalKey; if the UI is on a different language the wrong files are requested.

-`${base}/reversal/${defaultReversalKey}/${letter}-${String(fileIndex).padStart(3,'0')}.json`
+const langKey = getKeyByLang(selectedLanguage);
+`${base}/reversal/${langKey}/${letter}-${String(fileIndex).padStart(3,'0')}.json`

Coupled with the previous comment this will correctly download the right reversal index set.

Committable suggestion skipped: line range outside the PR's diff.

const data = await response.json();
const currentFileWords = Object.entries(data).map(([word, entries]) => {
return {
word,
indexes: entries.map((entry) => entry.index),
vernacularWords: entries
.map((entry) => {
const foundWord = vernacularWordsList.find(
(vw) => vw.id === entry.index
);
return null; // Return null for missing indexes
}
})
.filter((index) => index !== null), // Filter out null values
letter: letter
};
});
return foundWord
? {
name: foundWord.name,
homonymIndex: foundWord.homonym_index || 0
}
: null;
})
.filter(Boolean),
letter
};
});

currentFileWords.forEach((newWord) => {
const existingWord = newWords.find((w) => w.word === newWord.word);
if (existingWord) {
existingWord.indexes = [
...new Set([...existingWord.indexes, ...newWord.indexes])
];
} else {
newWords.push(newWord);
for (const newWord of currentFileWords) {
const existingWord = newWords.find((w) => w.word === newWord.word);
if (existingWord) {
existingWord.indexes = [
...new Set([...existingWord.indexes, ...newWord.indexes])
];
} else {
newWords.push(newWord);
}
}
});

fileIndex++;
} else {
moreFiles = false;
}
} catch (error) {
console.error('Error loading word data:', error);
moreFiles = false;
}
}

Expand Down Expand Up @@ -170,10 +169,12 @@
}
}


async function handleLetterChange(letter) {
selectedLetter = letter;
if (selectedLanguage === reversalLanguage) {
await fetchWords();
if (reversalLanguageList.includes(selectedLanguage)) {
await fetchWords(letter);
await tick();
}
scrollToLetter(letter);
}
Expand All @@ -182,7 +183,7 @@
selectedReversalLanguageStore.set(language);
selectedLanguage = language;
selectedLetter = currentAlphabet[0];
if (selectedLanguage != vernacularLanguage) {
if (selectedLanguage !== vernacularLanguage) {
fetchWords();
}
const scrollableDiv = document.querySelector('.flex-1.overflow-y-auto.bg-base-100');
Expand All @@ -193,6 +194,7 @@

let isFetching = false;


async function checkIfScrolledToBottom(event) {
if (isFetching) return;

Expand Down Expand Up @@ -264,7 +266,7 @@
alphabet={currentAlphabet}
{selectedLanguage}
{vernacularLanguage}
{reversalLanguage}
reversalLanguages={reversalLanguages.flatMap((lang) => Object.values(lang))}
{selectedLetter}
onSwitchLanguage={switchLanguage}
onLetterChange={handleLetterChange}
Expand Down
2 changes: 1 addition & 1 deletion src/routes/lexicon/+page.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export async function load({ fetch }) {
const vernacularLanguage = vernacularWritingSystem.displayNames.default;

const reversalWritingSystems = Object.entries(dictionaryConfig.writingSystems).filter(
([key, ws]) => !ws.type.includes('main')
([key, ws]) => 'reversalFilename' in ws
);

if (!reversalWritingSystems) {
Expand Down
Loading