Open page in llm copy button with dropdown#354
Conversation
Deploying head-start with
|
| Latest commit: |
c9059c4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0e4ff2d8.head-start.pages.dev |
| Branch Preview URL: | https://feat-open-page-in-llm-copy-b.head-start.pages.dev |
| setTimeout(() => this.#setLabel(this.#originalLabel), 2000); | ||
| }; | ||
|
|
||
| #buildCopyText = async (pageUrl: string): Promise<string> => { |
There was a problem hiding this comment.
When I click Copy page. The button says 'Failed to copy page'. This error however is a bit obscure and I think it would be nice to add a console error for this with more info about what went wrong.
|
|
||
| const { llmOptions, prompt } = Astro.props; | ||
|
|
||
| const popoverId = 'open-in-llm-popover'; | ||
| const pageUrl = Astro.url.href; | ||
|
|
||
| let searchText = prompt.replace('{ pageUrl }', pageUrl); |
There was a problem hiding this comment.
.replace('{ pageUrl }') is really fragile. I think you should implement a more robust way. Maybe a regex key? Or third party package?
Also this replaces the first instance of the { pageUrl }. What if someone wants to add multiple { pageUrl }{ pageUrl }{ pageUrl }? You could handle that through a regex or .replaceAll('{ pageUrl }') for example.
| Object.assign(popover.style, { | ||
| left: `${x}px`, | ||
| top: `${y}px`, | ||
| width: `${this.#buttonGroup!.offsetWidth}px`, |
There was a problem hiding this comment.
I think you should remove the width limitation since this causes every line to break because the width of the button set is always way smaller probably. If you're trying to prevent the box to flow off screen then I think you should alter your positioning to detect that and adjust the position.
3 participants
Changes
Associated issue
Resolves #341
How to test
Checklist