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

Identify unique pages across website where user needs to authenticate #4132

Closed
20 tasks done
SidharthBansal opened this issue Dec 5, 2018 · 100 comments
Closed
20 tasks done
Assignees
Labels
enhancement explains that the issue is to improve upon one of our existing features more-detail-please issue lacks proper description and perhaps needs code links or the location of the problem planning Planning issues!

Comments

@SidharthBansal
Copy link
Member

SidharthBansal commented Dec 5, 2018

We are creating modal for the login/signup process. We need to integrate it throughout the website to all the pages wherever login/signup is needed to use that area. Examples

screenshot 54 #4156

screenshot 55 #4157

image

screenshot 56

@SidharthBansal SidharthBansal added fto-candidate issues which are meant to be solved by first timers but aren't well-formatted yet more-detail-please issue lacks proper description and perhaps needs code links or the location of the problem labels Dec 5, 2018
@oorjitchowdhary
Copy link
Member

@publiclab/mentors what is the required submission for this task... I've to create a PR showing only the locations where OAuth can be implemented?

@SidharthBansal
Copy link
Member Author

NO PR REQUIRED.
Just tell me some places where we need login modal to be displayed on clicking the login/sign up.
Example we should have login modal on the question page. You know that OAuth modal is worked upon in other prs. So this issue's implementation will be after that issues completion. I hope this makes sense.

@oorjitchowdhary
Copy link
Member

Okay.. that seems good.. I'll then claim this one on gci in a while

@igniteeng000 igniteeng000 changed the title Identity different pages across website where user needs to authenticate first Identify different pages across website where user needs to authenticate first Dec 6, 2018
@mohitRJranjan
Copy link
Contributor

mohitRJranjan commented Dec 6, 2018

@oorjitchowdhary
Copy link
Member

@SidharthBansal

  • ask a question
  • comment
  • I have done activity column
  • Get Involved tab => Post your work
  • Like, follow, Flag as spam, Emoji on posts, comments and wikis
  • Tag click => Follow => Login/Sign up

@SidharthBansal
Copy link
Member Author

SidharthBansal commented Dec 6, 2018

@mohitRJranjan @oorjitchowdhary please don't write same points. Also, supply the route for it.
Thanks
Please write how that page is linked also. Like do we have a button for it or for the direct route, etc. Explain well.

@oorjitchowdhary
Copy link
Member

okay

@SidharthBansal
Copy link
Member Author

@oorjitchowdhary please organise in the form of checkboxes. Append the above changes also.
Thanks

@SidharthBansal SidharthBansal added this to the OAuth milestone Dec 6, 2018
@oorjitchowdhary
Copy link
Member

oorjitchowdhary commented Dec 6, 2018

@oorjitchowdhary
Copy link
Member

This works? @SidharthBansal

@SidharthBansal
Copy link
Member Author

SidharthBansal commented Dec 6, 2018

Like, follow, Flag as Spam, Emoji on comments and wikis
app/views/notes/_comment.html.erb
app/views/questions/_answers.html.erb
app/views/questions/_answer.html.erb
app/views/wiki/_header.html.erb

break this properly into different checkboxes and write sequence of flow for modal to get integrated like you write Get Involved Tab -> Post your work
Also append #4132 (comment) checkboxes with description.
Also write the urls. Thanks
Thanks

@oorjitchowdhary
Copy link
Member

oorjitchowdhary commented Dec 6, 2018

screenshot 54 #4156

screenshot 55 #4157

screenshot 56

@jonxuxu
Copy link
Member

jonxuxu commented Dec 6, 2018

@mohitRJranjan @oorjitchowdhary have you guys claimed this task? I see it's still available on the dashboard.

@SidharthBansal
Copy link
Member Author

@JonathanXu1 it is multiple instance count task. They have done the task. If you wish to do it you can claim and suggest me. Thanks

@SidharthBansal SidharthBansal added break-me-up break up for cleaner code separation, discrete tests, and, easier and iterative collaboration and removed fto-candidate issues which are meant to be solved by first timers but aren't well-formatted yet labels Dec 6, 2018
@SidharthBansal SidharthBansal changed the title Identify different pages across website where user needs to authenticate first Identify unique pages across website where user needs to authenticate first Dec 7, 2018
@SidharthBansal
Copy link
Member Author

@oorjitchowdhary Can you please help me for creating issues for your comment #4132 (comment)

@SidharthBansal
Copy link
Member Author

I have created some tasks so please see that your tasks will not repeat with my issues created.

@SidharthBansal
Copy link
Member Author

Just to ping all GCI students that creating n first timer issues will count as n tasks.
There are many activities which are small on GCI dashboard. So, please take those tasks. I created those activities so that you can work on more number of issues and get approval soon. And we can judge you better.

@jonxuxu
Copy link
Member

jonxuxu commented Dec 8, 2018

Here's some more areas to add logins:

@SidharthBansal is this sufficient?

@digitaldina
Copy link
Collaborator

@JonathanXu1 so for the store, I'm seeing this as the login:
screenshot from 2018-12-08 12-21-15

Do you mean that we should add Oauth to it?

@SidharthBansal
Copy link
Member Author

Great work!!!
It will be great if you can raise separate issues for these tasks and link them to this issue.

@SidharthBansal
Copy link
Member Author

@dinaelhanan let @JonathanXu1 raise the issue and link it here. We will converse about #4132 (comment) in that issue

@oorjitchowdhary
Copy link
Member

@JonathanXu1 so for the store, I'm seeing this as the login:
screenshot from 2018-12-08 12-21-15

Do you mean that we should add Oauth to it?

Wow.. that is just a great login page.. We could perhaps make it into a modal, add Oauths to it... And maybe also implement the same design on the publiclab.org login modal..

@SidharthBansal
Copy link
Member Author

SidharthBansal commented Dec 28, 2018 via email

@SidharthBansal SidharthBansal changed the title Identify unique pages across website where user needs to authenticate first Identify unique pages across website where user needs to authenticate Dec 29, 2018
@SidharthBansal
Copy link
Member Author

SidharthBansal commented Dec 29, 2018

  • flash message
  • add one row
    image

@SidharthBansal
Copy link
Member Author

@oorjitchowdhary did you see any other similar issues?

@oorjitchowdhary
Copy link
Member

oorjitchowdhary commented Dec 29, 2018

Found just one.. Should I create an issue for this @SidharthBansal

  • Log in button on /signup page

image4

But we're implementing the signup modal too right.. so is this required? Will we keep the log in button in the modal?

@kevinzluo
Copy link
Collaborator

@oorjitchowdhary
I believe @JonathanXu1 has linked the log in button on the signup modal to the log in modal.

Take a look at stable.publiclab.org

modallinked

@Rishabh-Kumar-Bothra
Copy link
Contributor

Rishabh-Kumar-Bothra commented Dec 29, 2018

@SidharthBansal
Copy link
Member Author

@kevinzluo @oorjitchowdhary Jonathan has made the links to both the modals and they are woking correctly. But on hte /signup and /login they are not redirecting to each other pages. We need to have submit a fix after forming issue for it. @oorjitchowdhary can you kindly solve this? Kevin is taking some other work so I don't want to burden him much. Thanks both of you.

@oorjitchowdhary
Copy link
Member

@SidharthBansal okay.. You mean to solve #4437 right?

@SidharthBansal
Copy link
Member Author

No, sorry for the misunderstanding. #4437 is being solved by @geekychaser.
@oorjitchowdhary you can create an issue for #4132 (comment) + #4132 (comment) and submit a PR for this. Once you finish this up please ping me I will assign you more tasks. I found some tasks upon which you will love to contribute.
Thanks @oorjitchowdhary

@SidharthBansal
Copy link
Member Author

@oorjitchowdhary you can definitely help people asking for help like in #4437 go ahead and help @geekychaser as he is stuck.
Thanks

@Rishabh-Kumar-Bothra
Copy link
Contributor

@SidharthBansal @oorjitchowdhary i think this can be solved now , i should post a PR after this get merged #4453

@SidharthBansal
Copy link
Member Author

@oorjitchowdhary some work is done in #4457. Please refer it before submitting a pr for #4132 (comment).
Thanks for the amazing work you are doing at public lab.

@oorjitchowdhary
Copy link
Member

@SidharthBansal I saw the stable PL status..
The log in on /signup doesn't work and sign up on /login doesn't work.. So should I link the respective modals to the buttons or the respective pages?

@kevinzluo
Copy link
Collaborator

You are right @oorjitchowdhary! #4457 also depends on this. Do we want the links at /signup and /login to open the modals or redirect to the pages? I am in favor of the modals.

@SidharthBansal
Copy link
Member Author

SidharthBansal commented Dec 31, 2018 via email

@oorjitchowdhary
Copy link
Member

oorjitchowdhary commented Dec 31, 2018 via email

@SidharthBansal
Copy link
Member Author

Hi, @oorjitchowdhary actually Public Lab is a website which we are maintaining since 10 yrs. So there are many places which contain link to login and signup as /signup or /login. In case we will remove these pages those links will stop working. Functionality is more important than redundancy. I liked your suggestion but we cannot implement it.
Thanks for the suggestion.

@SidharthBansal
Copy link
Member Author

SidharthBansal commented Dec 31, 2018

@SidharthBansal
Copy link
Member Author

@oorjitchowdhary we are creating the modals so that the person will not get derailed from the public labs while signup. I hope this solves your query.

@jywarren
Copy link
Member

jywarren commented Jan 2, 2019

This is a spectacular effort, folks! Very impressive to see. Do you think it's worth adding to the README or docs somewhere that this new class can be added to any link to require login first?

Thanks!!! 👍 🎉

@jywarren
Copy link
Member

jywarren commented Jan 2, 2019

Also it's useful to have distinct /login and /signup pages so that we can send people the link to those if necessary. Modals don't have a URL, although of course we could send them the URL of a page that they can't access, but that's a little more complex to remember.

@jywarren
Copy link
Member

jywarren commented Jan 2, 2019

I.e. "Oh, welcome! Try making an account at https://publiclab.org/signup"!

@SidharthBansal
Copy link
Member Author

Right now we are linking each Try making an account to signup modal and try logging in to login modal.
Thank you Jeff

@oorjitchowdhary
Copy link
Member

oorjitchowdhary commented Jan 3, 2019

@SidharthBansal
Copy link
Member Author

I hope we have created issues for all the buttons/links where ever we require the login modal. In case we will require any more button to be linked we can simply open up a first timer only labelled issue @jywarren so wrapping up here and closing this IMMENSE issue. Thanks to all the folks who helped PL for the login/sign up front end migration for better UI.

@jywarren
Copy link
Member

jywarren commented Jan 7, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement explains that the issue is to improve upon one of our existing features more-detail-please issue lacks proper description and perhaps needs code links or the location of the problem planning Planning issues!
Projects
None yet
Development

No branches or pull requests

9 participants