Conversation
…if there are more dom elements
prestoncraw
left a comment
prestoncraw
left a comment
There was a problem hiding this comment.
Whenever you are done addressing the comments on this could you do ctrl + k , ctrl + d to autoformat this file to fix the indentations, etc.?
| import { Portal } from 'react-portal' | ||
| import * as _ from 'lodash' | ||
|
|
||
| interface ILabelValue { |
There was a problem hiding this comment.
There is already an interface for this in the Gemstone namespace in application-typings.
| Valid={props.Valid} | ||
| Record={props.Record} | ||
| Setter={props.Setter} | ||
| Field={props.Field} |
There was a problem hiding this comment.
Not all props are being passed through, for example Label, Help, Feedback is missing here. Any prop that Input accepts we should also accept in this component and pass through to input unless it is something like Type which we know will be "text"(the default).
| } | ||
| }, 200); | ||
|
|
||
| const handleScroll = (_: Event) => { |
There was a problem hiding this comment.
No need for the event arg if we arent using it.
| const [autoCompleteOptions, setAutoCompleteOptions] = React.useState<ILabelValue[]>([]) | ||
| const [position, setPosition] = React.useState<Gemstone.TSX.Interfaces.IElementPosition>({ Top: 0, Left: 0, Width: 0, Height: 0 }); | ||
|
|
||
| const handleOptionClick = (evt: React.MouseEvent<HTMLTableRowElement, MouseEvent>, option: ILabelValue) => { |
There was a problem hiding this comment.
No need for the event arg if we arent using it.
| ) | ||
| } | ||
|
|
||
| export const handleAutoComplete = (inputString: string, autoCompletes: string[], autoCompleteSetter: React.Dispatch<React.SetStateAction<ILabelValue[]>>) => { |
There was a problem hiding this comment.
This looks to be the same function as the one used in AutoCompleteInput, they should be utilizing the same function.
| textarea.parentNode.appendChild(hiddenDiv); | ||
|
|
||
| // Get caret's vertical position relative to textarea | ||
| console.log(`text area: ${textarea.scrollLeft}, ${textarea.scrollTop}`); |
There was a problem hiding this comment.
There is multiple console.log in here for debugging purposes, these need to be removed.
| if (autoCompleteTextArea.current != null) { | ||
| if (textAreaElement.current == null) { | ||
| const textAreaComponent = autoCompleteTextArea.current.firstChild; | ||
| if (textAreaComponent == null) {return} | ||
| textAreaComponent.childNodes.forEach((element) => element.nodeName === 'TEXTAREA' ? textAreaElement.current = element as HTMLTextAreaElement : null) | ||
| } |
There was a problem hiding this comment.
This logic to assign a ref based on if the nodeName equals TextArea seems very fragile. I think the cleaner way to do this would be to just pass a ref to the TextArea component as an optional prop and have the TextArea component attach that ref to its internal TextArea element.
| const [ caret_X, caret_Y ] = getCaretPosition(textAreaElement.current); | ||
| console.log(caret_X, caret_Y, rect.top, rect.left) | ||
| // we want to offset the position based on the cursor position within the text area. | ||
| setPosition({ Top: rect.top + caret_Y - rect.bottom, Left: rect.left + caret_X, Width: rect.width, Height: rect.height }); |
There was a problem hiding this comment.
This positioning logic doesn't look right, during my testing the portal was being positioned at the very top of the screen.
Co-authored-by: Preston Crawford <78245034+prestoncraw@users.noreply.github.com> Signed-off-by: nbeatty-gpa <nbeatty@gridprotectionalliance.org>
Co-authored-by: Preston Crawford <78245034+prestoncraw@users.noreply.github.com> Signed-off-by: nbeatty-gpa <nbeatty@gridprotectionalliance.org>
Co-authored-by: Preston Crawford <78245034+prestoncraw@users.noreply.github.com> Signed-off-by: nbeatty-gpa <nbeatty@gridprotectionalliance.org>
|
these past few commits should address everything but the positioning issues. i've been trying to use this approach for positioning, and that's been working well over here |
2 participants
add AutoCompleteTextArea and AutoCompleteInput to add autocomplete functionality for given variables wrapped in curly brackets.