Skip to content

Conversation

@BaaniLeen
Copy link
Member

@BaaniLeen BaaniLeen commented Jul 26, 2018

Description

As a contributor,
I need create a section on the newcomers page for first timer issues.

Fixes #252

TIme Taken:
24 hours (6 hours research + 2 hours implementation + 2 hours resolving merge conflicts on my local branch - all my PRs on the newcomers page is leading to a lot of conflicts + 3 hours reading about the iframe issue, Bethany mentioned about issues not being clickable - blank frame opening up+ 7 hours implementing a modular implementation, getting error at Array implementation, getting stuck at the unsafe URL error, looked up and tried solutions (mentioned on slack: https://systers-opensource.slack.com/archives/C99N86T43/p1532619465000180) + 3 hours - implemented less modular but working code using tabsets + 1 hour - added all projects info acc to their technology stack manually)

Type of Change:

  • Code
  • Quality Assurance
  • User Interface
  • Outreach
  • Documentation

Code/Quality Assurance Only

  • Bug fix (non-breaking change which fixes an issue)
  • This change requires a documentation update (software upgrade on readme file)
  • New feature (non-breaking change which adds functionality pre-approved by mentors)

How Has This Been Tested?

Ran it locally

Checklist:

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials
  • I have commented my code or provided relevant documentation, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • Any dependent changes have been merged

Code/Quality Assurance Only

  • My changes generate no new warnings
  • My PR currently breaks something (fix or feature that would cause existing functionality to not work as expected)
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been published in downstream modules

@bethanyr
Copy link
Contributor

When I click on a newcomer issue, then I just see a blank area in the iframe.

@BaaniLeen
Copy link
Member Author

BaaniLeen commented Jul 27, 2018

@bethanyr It works properly when opened in a new tab, but yes, it cannot open the link within the iframe and gives this error: Refused to display 'https://github.com/systers/systers.github.io/issues/243' in a frame because an ancestor violates the following Content Security Policy directive: "frame-ancestors 'none'".
Looked up about it.
This error is displaying due to GitHub's Content Security Policy of not allowing it's view to be displayed within an iframe. : cypress-io/cypress#1555.
It is applicable to us too: https://stackoverflow.com/questions/38535491/trying-to-render-i-frame-ancestor-violates-the-following-content-security-polic

@bethanyr
Copy link
Contributor

@BaaniLeen - now there's conflicts in this PR

@BaaniLeen
Copy link
Member Author

@bethanyr Resolved the merge conflict :)

@ancyp
Copy link
Contributor

ancyp commented Jul 27, 2018

There needs to be some indication that it needs to be opened in a new page.

We believe in inclusivity, diversity, and transparency. We value open source citizenship and collaborative involvement.
</p>
</div>
<div class="row justify-content-center mb-3">
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought you had a previous PR that removed the map from the newcomer's page? Or was it a different page?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. Before it got merged, I created this branch and started working on this PR. So will have to manually resolve these merge conflicts perhaps.

Copy link
Contributor

Choose a reason for hiding this comment

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

There aren't merge conflicts showing, so when this branch gets merged I think it will re-add the map into the project. You should just go ahead and delete the map out of this code again.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh okay. Sure. Doing it.

Copy link
Member Author

Choose a reason for hiding this comment

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

@bethanyr Done. :)

<!-- Peace Corps Projects Note-->
<div class="row text-center">
<h1 class="mx-auto mt-5 mb-5">Sysbot</h1>
<iframe class="iframe-style" src="https://publiclab.github.io/community-toolbox/examples/embed.html#o=systers&r=sysbot"></iframe>
Copy link
Contributor

Choose a reason for hiding this comment

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

are you planning to only show first-timer issues or all issues for a repo?

Copy link
Member Author

Choose a reason for hiding this comment

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

I am planning to show first-timers-only issues for all repos, but since right now, none of them have the "first-timers-only" label(other than systers.github.io) but rather have the "First Timers Only" tag, with the spaces, which I did try to work around with using %20 instead of spaces, but did not work. So, I have requested May that we introduce first-timers-only labels, if possible in all the repos, was waiting to discuss with all of you in the All teams meeting today.
Thus, since no issues exist current for the "first-timers-only" label, so in order to show a proper demo, I have not mentioned the label tag yet in the URL.
What are your views about it? Shall I mention the "first-timers-only" label?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's fine for the demo.

@ancyp
Copy link
Contributor

ancyp commented Jul 29, 2018

I don't think we decided on a solution to the empty iframe in the meeting. Is there a way to do it other than iframes?
If not can we make it a separate issue and merge this?

@BaaniLeen
Copy link
Member Author

@ancyp This was the discussion wherein we decided to use iframes. I would suggest we close this issue for now, and create a separate issue for the empty iframe thing. What do you say, @bethanyr @janiceilene ? :)

@bethanyr
Copy link
Contributor

Let's go ahead and create another issue to implement without iframes so you can control how the links open and we can merge this item in for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants