docs: add new LLM dropdown #3267
Conversation
birosrichard
added
the
t-web
katzino
left a comment
katzino
left a comment
There was a problem hiding this comment.
Pre-approving, smaller nits but nothing major 💪
|
|
||
| const handleKeyDown = useCallback( | ||
| (event) => { | ||
| if (event.key === 'Enter' || event.key === ' ') { |
There was a problem hiding this comment.
I would prefer this just for better understanding
Edit: I see we use this across the codebase, nevermind 😅
| if (event.key === 'Enter' || event.key === ' ') { | |
| if (event.key === 'Enter' || event.code === "Space") { |
| aria-controls={'llm-menu'} | ||
| /> | ||
| {isOpen && ( | ||
| <div className={styles.menuDropdown} role="menu" id={'llm-menu'}> |
There was a problem hiding this comment.
| <div className={styles.menuDropdown} role="menu" id={'llm-menu'}> | |
| <div className={styles.menuDropdown} role="menu" id="llm-menu"> |
| onKeyDown={handleKeyDown} | ||
| aria-haspopup="menu" | ||
| aria-expanded={isOpen} | ||
| aria-controls={'llm-menu'} |
There was a problem hiding this comment.
| aria-controls={'llm-menu'} | |
| aria-controls="llm-menu" |
| {copyingStatus === 'loading' && <LoaderIcon size={16} />} | ||
| {copyingStatus === 'copied' && <CheckIcon size={16} />} | ||
| {copyingStatus === 'idle' && <CopyIcon size={16} />} |
There was a problem hiding this comment.
Might be nicer to have a map for each state and render a single node based on the value
barjin
left a comment
barjin
left a comment
There was a problem hiding this comment.
(Older) versioned pages do not have the markdown alternatives, so clicking the markdown button e.g. on this page will result in 404 (https://crawlee.dev/js/docs/3.11/examples/accept-user-input.md doesn't exist).
Aside from that, I found some visual bugs:
| const markdownUrl = getMarkdownUrl(window.location.href); | ||
|
|
||
| try { | ||
| window.open(markdownUrl, '_blank'); |
There was a problem hiding this comment.
Future update idea: can we render actual links (anchor elements) here?
|
nit: anything with the |
closes: apify/apify-web#5475 Copied logic from apify/crawlee#3267
https://github.com/apify/apify-web/issues/5475
Adding a new LLM dropdown.