Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions src/app/PageRoutes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import DocsPage from '@pages/DocsPage';
import IntroPage from '@pages/IntroPage';
import LuckySearchPage from '@pages/LuckySearchPage';
import PrivacyPolicyPage from '@pages/PrivacyPolicyPage';
import CodeLookupPage from '@pages/code-lookup/CodeLookupPage';

export default function PageRoutes() {
return (
Expand All @@ -23,6 +24,7 @@ export default function PageRoutes() {
<Route path={LangNavPageName.About} element={<AboutPage />} />
<Route path={LangNavPageName.PrivacyPolicy} element={<PrivacyPolicyPage />} />
<Route path={LangNavPageName.DataCoverage} element={<DataCoveragePage />} />
<Route path={LangNavPageName.Codes} element={<CodeLookupPage />} />

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for working with the merge conflict I JUST gave you.

</Routes>
</>
);
Expand Down Expand Up @@ -55,4 +57,5 @@ export enum LangNavPageName {
About = 'about',
PrivacyPolicy = 'privacy-policy',
DataCoverage = 'data-coverage',
Codes = 'codes',
}
73 changes: 73 additions & 0 deletions src/pages/code-lookup/CodeLookupPage.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
.code-lookup-container {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hmmmm.

While I was at Facebook we deprecated .css files in favor of providing the styles directly in react components. The main reasons was to colocate styles with components and reduce file fragmentation (especially since there are different TTL for .css and .js files so the resources could become desynchronized). Instead we use inline styles and often even separate "style" components (see DocsComponents.tsx) from the components with the interactions/data.

So I was going to tell you to do that instead... but, ideally I would provide evidence for that design choice so I looked it up and...

It looks like the state of the art is actually doing what you are doing, .css files. So I have no strong opinion. Also it's confusing that we still have @pages/dataviews/styles.css -- that's really a relic of the early development of LangNav.

max-width: 900px;
margin: 0 auto;
padding: 2em 1.5em;
}

.code-lookup-container h2 {
margin-top: 0;
}

.code-lookup-textareas {
display: flex;
gap: 1em;
margin-top: 1em;
}

@media (max-width: 640px) {
.code-lookup-textareas {
flex-direction: column;
}
}

.code-lookup-column {
flex: 1;
display: flex;
flex-direction: column;
gap: 0.5em;
}

.code-lookup-column label {
font-weight: 600;
}

.code-lookup-textarea {
width: 100%;
min-height: 220px;
font-family: 'Courier New', Courier, monospace;
font-size: 0.95em;
line-height: 1.6;
padding: 0.75em;
border: 1px solid var(--color-text-secondary);
border-radius: 6px;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

To stay consistent with the rest of the library, use 0.25em, 0.5em or 1em for border radius

background-color: var(--color-background);
color: var(--color-text);
Comment on lines +43 to +44

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

thanks for using the color variables.

resize: vertical;
box-sizing: border-box;
}

.code-lookup-textarea:focus {
outline: 2px solid var(--color-button-primary);
border-color: var(--color-button-primary);
}

.code-lookup-textarea[readonly] {
background-color: color-mix(in srgb, var(--color-button-secondary), var(--color-background) 50%);
}

.code-lookup-actions {
display: flex;
gap: 0.75em;
margin-top: 1em;
flex-wrap: wrap;
}

.code-lookup-status {
margin-top: 1em;
font-size: 0.9em;
color: var(--color-text-secondary);
}

.code-lookup-status .warning {
color: var(--color-text-orange);
}
21 changes: 21 additions & 0 deletions src/pages/code-lookup/CodeLookupPage.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import React, { Suspense } from 'react';

import Loading from '@widgets/Loading';

const PageParamsProvider = React.lazy(() => import('@features/params/PageParamsProvider'));
const DataProvider = React.lazy(() => import('@features/data/context/DataProvider'));
const CodeLookupPageBody = React.lazy(() => import('./CodeLookupPageBody'));

const CodeLookupPage: React.FC = () => {
return (
<Suspense fallback={<Loading />}>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wrap this in <PageContainer title="Language Code Lookup"> & add the description here so while the suspense is in the loading state people know they are in the right page.

<PageParamsProvider>
<DataProvider>
Comment on lines +12 to +13

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

While you were working on this, we changed it so PageParamsProvider & DataProvider are provided in by the parent file App.tsx.

Note: There is a time where DataProvider will be empty (while it is loading) but I think it's okay.

Suggested change
<PageParamsProvider>
<DataProvider>

<CodeLookupPageBody />
</DataProvider>
</PageParamsProvider>
</Suspense>
);
};

export default CodeLookupPage;
89 changes: 89 additions & 0 deletions src/pages/code-lookup/CodeLookupPageBody.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import { useDataContext } from '@features/data/context/useDataContext';
import React, { useState } from 'react';
import './CodeLookupPage.css';
import { batchLookup, BatchMatchResult } from './batchMatchLogic';

const CodeLookupPageBody: React.FC = () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Eventually, move this to the widgets folder.

const { languagesInSelectedSource } = useDataContext();
const [inputText, setInputText] = useState('');
const [outputText, setOutputText] = useState('');
const [statusMessages, setStatusMessages] = useState<string[]>([]);
const [copied, setCopied] = useState(false);

const isLoading = languagesInSelectedSource.length === 0;

function handleLookup() {
const lines = inputText.split('\n');
const results: BatchMatchResult[] = batchLookup(lines, languagesInSelectedSource);

const codes = results.map((r) => r.code);
setOutputText(codes.join('\n'));

const warnings: string[] = [];
results.forEach((r, i) => {
if (r.warning) {
warnings.push(`Line ${i + 1}: ${r.warning}`);
}
});
setStatusMessages(warnings);
setCopied(false);
}

async function handleCopy() {
try {
await navigator.clipboard.writeText(outputText);
setCopied(true);
setTimeout(() => setCopied(false), 2000);
} catch { }
}
Comment on lines +32 to +38

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

These functions rebuild every re-render, but useCallback will prevent that. Also, be careful when adding page timers (I don't always do this but I intend to) -- you should make a way for it to destroy itself if the event is terminated early. It's not as a big deal for timers, but it is for things like keyboard event listeners (since if you don't clean the up, you will end up creating a bunch of keyboard event listeners that never are terminated and could slow down perf).

Suggested change
async function handleCopy() {
try {
await navigator.clipboard.writeText(outputText);
setCopied(true);
setTimeout(() => setCopied(false), 2000);
} catch { }
}
const handleCopy = useCallback(
async () => {
navigator.clipboard.writeText(outputText);
setCopied(true);
const timer = setTimeout(() => setCopied(false), 2000);
return () => clearTimeout(timer);
},
[outputText],
);


Check failure on line 39 in src/pages/code-lookup/CodeLookupPageBody.tsx

View workflow job for this annotation

GitHub Actions / format

Empty block statement
return (
<div className="code-lookup-container">
<h2>Language Code Lookup</h2>
<p>
Enter language names below (one per line) and click <strong>Look Up</strong> to get their
language codes. Example: <code>Punjabi</code> &gt;&gt; <code>lah/pan</code>.
</p>
<div className="code-lookup-textareas">
<div className="code-lookup-column">
<label htmlFor="batch-input">Language Names</label>
<textarea

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Instead of a manual button, its a better UX to have the algorithm automatically run after a person enters data. Options:

onKeyDown (with an if statement for event.key === enter)
onBlur (when they click away after having looking at the data)
onChange (this probably would cause too many calls, you could debounce it though (only call it on a timeout))

id="batch-input"
className="code-lookup-textarea"
placeholder={'Example:\nUrdu\nPunjabi\nHindko'}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

"Example:" is obvious

Suggested change
placeholder={'Example:\nUrdu\nPunjabi\nHindko'}
placeholder={'Urdu\nPunjabi\nHindko'}

value={inputText}
onChange={(e) => setInputText(e.target.value)}
/>
</div>
<div className="code-lookup-column">
<label htmlFor="batch-output">Language Codes</label>
<textarea

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

add a placeholder here to mimic the example you used above.

id="batch-output"
className="code-lookup-textarea"
readOnly
value={outputText}
/>
</div>
</div>
<div className="code-lookup-actions">
<button className="primary" onClick={handleLookup} disabled={isLoading || inputText.trim() === ''}>
{isLoading ? 'Loading data...' : 'Look Up'}
</button>
{outputText && (
<button onClick={handleCopy}>{copied ? 'Copied!' : 'Copy Output'}</button>
)}
</div>
{statusMessages.length > 0 && (
<div className="code-lookup-status">
{statusMessages.map((msg, i) => (
<div key={i} className="warning">
{msg}
</div>
))}
</div>
)}
</div>
);
};

export default CodeLookupPageBody;
Loading
Loading