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

Update profile icons to open in new tab on Wins page #2267

Closed
2 tasks done
Tracked by #2135
arghmatey opened this issue Sep 15, 2021 · 7 comments · Fixed by #2621
Closed
2 tasks done
Tracked by #2135

Update profile icons to open in new tab on Wins page #2267

arghmatey opened this issue Sep 15, 2021 · 7 comments · Fixed by #2621
Assignees
Labels
Complexity: Small Take this type of issues after the successful merge of your second good first issue Feature: Refactor JS / Liquid Page is working fine - JS / Liquid needs changes to become consistent with other pages P-Feature: Wins Page https://www.hackforla.org/wins/ role: front end Tasks for front end developers size: missing Status: Updated No blockers and update is ready for review

Comments

@arghmatey
Copy link
Contributor

arghmatey commented Sep 15, 2021

Overview

As a user while on the wins page, clicking on a Linkedin or Github link on a modal should open that profile page in a new tab. Currently - when clicking on a submission's Linkedin or Github on a modal window, the link opens in the current tab.

The modal links are dynamically created, and the target attribute needs to be added to that function.

Action Items

  • Locate the makeIcon function in our wins javascript
  • Set a new target attribute to the icon being created with a value of _blank

Resources/Instructions

Wins Page
wins.html

@arghmatey arghmatey added role: front end Tasks for front end developers Feature: Refactor JS / Liquid Page is working fine - JS / Liquid needs changes to become consistent with other pages P-Feature: Wins Page https://www.hackforla.org/wins/ Complexity: Small Take this type of issues after the successful merge of your second good first issue labels Sep 15, 2021
@github-actions

This comment has been minimized.

@arghmatey arghmatey changed the title Update modal icons to open in new tab on Wins page Update profile icons to open in new tab on Wins page Sep 26, 2021
@Johnnie007 Johnnie007 self-assigned this Dec 9, 2021
@Johnnie007
Copy link
Contributor

Johnnie007 commented Dec 9, 2021

@Johnnie007
Copy link
Contributor

Are we able to confirm this? @macho-catt

@github-actions github-actions bot added the Status: Updated No blockers and update is ready for review label Dec 10, 2021
@Johnnie007
Copy link
Contributor

This task has been resolved when the Wins page was converted to use the Jekyll Template. The template includes the attribute and value for target="_blank" that would open up a new tab on the web browser when the icon is clicked. This code has already been merged to the main branch, and no additional work needs to be done on this issue.

LINK HERE: https://github.com/hackforla/website/blob/gh-pages/_includes/wins-page/wins-card-template.html#:~:text=%3Cspan%20class%3D%22wins%2Dcard%2Dicons,%3C/span%3E

The issue just needs to be closed.

@macho-catt
Copy link
Member

So the linked code does open up a new tab, but I believe the problem is when you click on "see more" when on desktop view. This opens up a modal, and when you click on linkedin/github on the modal it should open a new tab (currently it opens on the same tab).

As to why the class "needs to be deleted" according to the comment, I don't really know why. I would say the previous developer who worked on this wanted to rework the class but didn't have the time to.... I would say to either ignore that comment or remove that comment for now.

I think the trick for this is to update the function below to have a new attribute for taret="_blank":

website/assets/js/wins.js

Lines 423 to 428 in de29138

function makeIcon(href, parent, className, src) {
let icon = makeElement('a', parent, 'wins-card-icon');
icon.setAttribute("href", href);
let iconImg = makeElement('img', icon, className);
iconImg.setAttribute("src", src);
}

@Johnnie007
Copy link
Contributor

Availability for this week:
Thursday 9 pm - 10 pm EST
My estimated ETA for completing this issue:
15 mins

@SAUMILDHANKAR
Copy link
Member

@Johnnie007: Just wanted to share a point from our Sunday team meeting. We are now requesting all developers to add eta and availability at the time of picking up an issue or when you are assigned to an issue. Just wanted to let you know. Thanks for your awesome work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complexity: Small Take this type of issues after the successful merge of your second good first issue Feature: Refactor JS / Liquid Page is working fine - JS / Liquid needs changes to become consistent with other pages P-Feature: Wins Page https://www.hackforla.org/wins/ role: front end Tasks for front end developers size: missing Status: Updated No blockers and update is ready for review
Projects
Development

Successfully merging a pull request may close this issue.

6 participants