Skip to content
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

Feat: Copy to Clipboard for FAQ #1303

Merged
merged 11 commits into from
Feb 9, 2024

Conversation

thisisobate
Copy link
Contributor

@thisisobate thisisobate commented Feb 2, 2024

Todos

  • POC
  • Fix style
  • Clean up code

Fixes #1274

Copy link

netlify bot commented Feb 2, 2024

Deploy Preview for keda ready!

Name Link
🔨 Latest commit 5c201c1
🔍 Latest deploy log https://app.netlify.com/sites/keda/deploys/65c62bc766c85900083a82bc
😎 Deploy Preview https://deploy-preview-1303--keda.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

github-actions bot commented Feb 2, 2024

Thank you for your contribution! 🙏 We will review your PR as soon as possible.

While you are waiting, make sure to:

  • Add your contribution to all applicable KEDA versions
  • GitHub checks are passing
  • Is the DCO check failing? Here is how you can fix DCO issues

Learn more about:

Signed-off-by: thisisobate <[email protected]>
Signed-off-by: thisisobate <[email protected]>
@thisisobate thisisobate marked this pull request as ready for review February 5, 2024 20:54
@thisisobate thisisobate requested a review from a team as a code owner February 5, 2024 20:54
Copy link
Member

@tomkerkhove tomkerkhove left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a ton!

Just a few suggestions:

  • Can we make the button less explicit? It's pretty invasive but it might be me, @zroubalik/@JorTurFer ?

image

  • When copying a link and going to the URL, the item does not expand so I still have to look for it, can we auto-expand them?

  • Can we do the same for troubleshooting?
    image

@JorTurFer
Copy link
Member

I agree with all your points 😄

Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with @tomkerkhove

@thisisobate
Copy link
Contributor Author

@tomkerkhove I suggest we do the troubleshooting part in a separate PR.

@tomkerkhove
Copy link
Member

@tomkerkhove I suggest we do the troubleshooting part in a separate PR.

Sounds good!

@tomkerkhove
Copy link
Member

The expanding is great! However, if I wouldn't now there was a button I would not see it since it requires a hover:
image

Can we maybe just get rid of the full blue background and always show a subtle icon?

@thisisobate
Copy link
Contributor Author

@tomkerkhove How about this?

Screenshot 2024-02-07 at 15 01 57

Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This icon looks better, though it might be just me but the icon gives me impression that I am about to copy some piece of text or code, not a link. Do we have an icon like this?
Screenshot 2024-02-07 at 15 10 26

@thisisobate
Copy link
Contributor Author

@zroubalik Yes, we do.
I agree with you on the icons.
Will substitute the clone icon with the link icon.

@tomkerkhove
Copy link
Member

Works like a charm, thanks! However, it no longer auto-expands which it used to do though :(

@thisisobate
Copy link
Contributor Author

@tomkerkhove Actually, it does expand. It looks like you tried loading the FAQ link in the same tab.
To test appropriately, copy the link and load it in a separate tab. That should work.

@tomkerkhove
Copy link
Member

Hm, why would that not work? It's not a blocker but curious though

@tomkerkhove tomkerkhove requested a review from zroubalik February 9, 2024 13:42
@thisisobate
Copy link
Contributor Author

Hm, why would that not work? It's not a blocker but curious though

So here's a bit of an explanation of why it won't work:,
By default, pages are cached when loaded in the browser tab. Now, when you try to navigate to an internal link (FAQ link) in this case, it won't cause a reload or rerender because it's an internal link; thus the reason why you can't see a change when you load the link in the same tab.

Hope that helps.

@tomkerkhove tomkerkhove merged commit a82e9fe into kedacore:main Feb 9, 2024
8 checks passed
@tomkerkhove
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide "Copy Link" button for FAQ questions
4 participants