Conversation
…t grows and becomes scrollable
…button is inside the chatinputbox + the msg in textarea is cleared immediately after msg is sent
…re new messages below that can't be seen
|
tested all changes and looks good so far- especially the new suggestion seems to work as expected, returning empty list when none is selected. (chat user icon thing may still need attention- I'll follow up after taking a closer look at it) |
kcarnold
left a comment
kcarnold
left a comment
There was a problem hiding this comment.
I've only looked at the code, not actually tried it.
| - Vary the sentence structure, word choice, and tone across the three options. | ||
| - Maintain the writer's overall voice and style. | ||
| - Each rewording should be approximately the same length as the original text. | ||
| - If no text is selected, return an empty list. |
There was a problem hiding this comment.
How about use the word or phrase (up to N words) nearest the cursor?
This prompt is ambiguous about whether it's ok or desired to reword other parts of the sentence (you say both "selected text" and "sentence structure"). Maybe that's ok?
There was a problem hiding this comment.
I've removed "sentence structure" from the prompt and gave explicit instruction to only rephrase the selected text.
There was a problem hiding this comment.
Can you show some examples? (Maybe we could have an EXAMPLES.md or even an examples folder where we show examples of good (and not so good) outputs?)
| if prompt_name == "example_rewording" and doc_context.selectedText.strip() == "": | ||
| return GenerationResult( | ||
| generation_type=prompt_name, | ||
| result="Please select some text to get rewording suggestions.", |
There was a problem hiding this comment.
does this display correctly?
| const scrollToBottom = useCallback(() => { | ||
| const container = messagesContainerRef.current; | ||
| if (container) { | ||
| container.scrollTop = container.scrollHeight; |
There was a problem hiding this comment.
Could use smooth scroll. I do this here: https://github.com/kcarnold/live-notes/blob/908146dc819f0994fca011932a788e1274ac9fa6/src/reactUtils.tsx#L30 (I forget exactly why I used a target element at the bottom instead of clientHeightlike you do here).
| const messagesContainerRef = useRef<HTMLDivElement>(null); | ||
| const [showScrollButton, setShowScrollButton] = useState(false); | ||
|
|
||
| const handleScroll = useCallback(() => { |
There was a problem hiding this comment.
maybe have a discussion around resize observers, since resizing the window could trigger need to scroll without actually firing onScroll.
| type="button" | ||
| title="Scroll to bottom" | ||
| onClick={scrollToBottom} | ||
| className="absolute bottom-[8px] left-1/2 -translate-x-1/2 bg-white border border-gray-300 rounded-full p-[6px] cursor-pointer shadow-md transition duration-150 hover:bg-gray-100" |
There was a problem hiding this comment.
curiosity, what does starting a classname with - do?
There was a problem hiding this comment.
It looks like tailwind allows - to apply negative version of that; i.e., that would be a negative transform.
|
To be clear, I'm ok with merge, my comments are just ideas/suggestions/low-priority issues. |
4 participants
Features and Fixes:
For PR review, changes are obvious when looking side-by-side with current UX.