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

Refactor computeLink function and event listeners, fixes #30 #32

Merged
merged 2 commits into from
Feb 12, 2024

Conversation

shaal
Copy link
Collaborator

@shaal shaal commented Feb 11, 2024

I updated the Pages settings of my repo, you can test it here -
https://shaal.github.io/ddev-gitpod-launcher/

Notice that with this PR every keystroke immediately updates the link.

Fixes

- Refactor computeLink function to improve readability and security:
  - Replace innerHTML with textContent when setting the ddevLinkElement content for enhanced security and performance.
- Initialize URL link on page load by calling computeLink function.
- Streamline event listener attachment for form elements with a 'name' attribute within the 'target' element.
- Attach updateArtifacts function as an event listener to the 'ddev-repo' input field to handle updates.
@rfay
Copy link
Member

rfay commented Feb 11, 2024

Thanks so much for this. Does this mean that in the entire history of this ddev-gitpod-launcher it always failed and nobody ever reported the failure?

@rfay
Copy link
Member

rfay commented Feb 11, 2024

Gave you full privs on this repo.

@rfay
Copy link
Member

rfay commented Feb 11, 2024

But of course please see much simpler

Also please provide manual testing instructions.

@shaal
Copy link
Collaborator Author

shaal commented Feb 12, 2024

@rfay I added now testing steps in the description

@rfay rfay changed the title Refactor computeLink function and event listeners Refactor computeLink function and event listeners, fixes #30 Feb 12, 2024
@rfay rfay merged commit 0d98fb5 into ddev:main Feb 12, 2024
@ultimike
Copy link
Contributor

Thanks to you both for fixing this :)

-mike

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.

None yet

3 participants