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

FIX : unintended opening of urls #407

Merged
merged 3 commits into from
Sep 11, 2021

Conversation

JooHyukKim
Copy link
Contributor

Hi, I was working on the achivementCard.js file to invalidate window.open action gets empty. Thought it will be okay to PR.

There are couple of things to point out here.

#1

First what I found as trouble here was that not always you have url for a specific tag, or specific card.
Sometimes you just want to place keywords.
Not link to somewhere.
Even the master branch had some empty urls.
Made me realize it IS okay to not have urls sometimes.
Below are cases I tested and worked out alright.

  1. with actual url
  2. with empty string as url property
  3. no url property

Screen Shot 2021-09-11 at 7 49 34 PM

#2

Second, some onClick callbacks did not even match the function names, So I fixed em up also.

#3

Lastly, I really appreciate your project.

Lemme know if anything,
cheers from Seoul, Korea

@vercel
Copy link

vercel bot commented Sep 11, 2021

Someone is attempting to deploy a commit to a Personal Account owned by @saadpasta on Vercel.

@saadpasta first needs to authorize it.

@vercel
Copy link

vercel bot commented Sep 11, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/saadpasta/developer-folio/FXbpscSyMitBWJtDsCXGu44eFXoL
✅ Preview: https://developer-folio-git-fork-joohyukkim-kjh-minor-fix-saadpasta.vercel.app

@saadpasta saadpasta merged commit b450422 into saadpasta:master Sep 11, 2021
@saadpasta
Copy link
Owner

@allcontributors please add @JooHyukKim for code

@allcontributors
Copy link
Contributor

@saadpasta

I've put up a pull request to add @JooHyukKim! 🎉

Mehranmzn pushed a commit to Mehranmzn/mehranmzn.github.io that referenced this pull request Sep 13, 2024
* FIX : AchievementCard, url validation added to not open empty window

* FIX : function openUrlInNewTab() not invoked properly

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
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.

2 participants