-
Notifications
You must be signed in to change notification settings - Fork 13
Feature/code lookup #563
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.
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
base: master
Are you sure you want to change the base?
Feature/code lookup #563
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| .code-lookup-container { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmmmm. While I was at Facebook we deprecated 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, |
||
| 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; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
| } | ||
| 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 />}> | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrap this in |
||||||
| <PageParamsProvider> | ||||||
| <DataProvider> | ||||||
|
Comment on lines
+12
to
+13
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||||||
| <CodeLookupPageBody /> | ||||||
| </DataProvider> | ||||||
| </PageParamsProvider> | ||||||
| </Suspense> | ||||||
| ); | ||||||
| }; | ||||||
|
|
||||||
| export default CodeLookupPage; | ||||||
| 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 = () => { | ||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| 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> >> <code>lah/pan</code>. | ||||||||||||||||||||||||||||||||||
| </p> | ||||||||||||||||||||||||||||||||||
| <div className="code-lookup-textareas"> | ||||||||||||||||||||||||||||||||||
| <div className="code-lookup-column"> | ||||||||||||||||||||||||||||||||||
| <label htmlFor="batch-input">Language Names</label> | ||||||||||||||||||||||||||||||||||
| <textarea | ||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) |
||||||||||||||||||||||||||||||||||
| id="batch-input" | ||||||||||||||||||||||||||||||||||
| className="code-lookup-textarea" | ||||||||||||||||||||||||||||||||||
| placeholder={'Example:\nUrdu\nPunjabi\nHindko'} | ||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "Example:" is obvious
Suggested change
|
||||||||||||||||||||||||||||||||||
| value={inputText} | ||||||||||||||||||||||||||||||||||
| onChange={(e) => setInputText(e.target.value)} | ||||||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||
| <div className="code-lookup-column"> | ||||||||||||||||||||||||||||||||||
| <label htmlFor="batch-output">Language Codes</label> | ||||||||||||||||||||||||||||||||||
| <textarea | ||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||||||||||||||||||||||||||||||||||
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.
Thanks for working with the merge conflict I JUST gave you.