Plots2: Combine app/views/user_sessions/_new.html.erb and new.html.erb

Created on 29 Dec 2018  Â·  24Comments  Â·  Source: publiclab/plots2

These two files are very similar except for the form url. Please keep new.html.erb but have it render _new.html.erb as a partial.

We can figure out which url to use possibly by passing a local into the partial.

enhancement gsoc rgsoc

Most helpful comment

@Dsxv will be willing to work on this issue as your next issue?
You were asking for some help. In case you need more help feel free to ping me, @jywarren or gaurav.
Thanks

All 24 comments

Also #4457 will introduce another difference between the two, so need to watch out for that too

@kevinzluo would you like to solve it yourself? Thanks!

@gauravano I can work on this. However, could I go ahead and fix #4457 even though it is an fto since that is related to this?

@Dsxv will be willing to work on this issue as your next issue?
You were asking for some help. In case you need more help feel free to ping me, @jywarren or gaurav.
Thanks

Yes I'd love to work on this ! Thanks for asking :D

You have to make a partial for the common code so that the readability can be increased

Hi! I wanted to ask after reducing the code , which one of the files should I keep ,file : new.html.erb or
partial : _new.html.erb
thanks!

_new.html.erb i guess.
Please see all the places where we are using this file. If we are removing
it then we need to check that all other places correctly work

On Tue, Jan 8, 2019 at 10:26 PM Pawan Shukla notifications@github.com
wrote:

Hi! I wanted to ask after reducing the code , which one of the files
should I keep ,file : new.html.erb or
partial : _new.html.erb
thanks!

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/plots2/issues/4456#issuecomment-452372464,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AUACQ3Zd0KzElU4XsXR9IBr8TcOgIHOCks5vBM3XgaJpZM4Zktm-
.

@SidharthBansal I just removed the file contents of new.html.erb and placed render to the partial _new.html.erb in it but everything is still working fine , certainly I am missing something here
rake test --all is not able to find anything related to this too , I also signed up with Oauth and the results are same . I am not sure here , pls Help !
thanks !

Please check following
Edit profile page
Profile page
Login modal
Signup modal
Login page
Signup modal
Each of above should work perfectly then we are good. I also think you are
missing something.

On Wed, Jan 9, 2019, 1:59 PM Pawan Shukla <[email protected] wrote:

@SidharthBansal https://github.com/SidharthBansal I just removed the
file contents of new.html.erb and placed render to the partial
_new.html.erb in it but everything is still working fine , certainly I am
missing something here
rake test --all is not able to find anything related to this too , I also
signed up with Oauth and the results are same . I am not sure here , pls
Help !
thanks !

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/plots2/issues/4456#issuecomment-452612413,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AUACQ7HfQ31H4hYxRUFX0KXMJ2vHnicKks5vBaiAgaJpZM4Zktm-
.

I'm going to post a gif while working on the localhost , will that do ?
thanks!

No need to check oauth. I know they will work.
Yeah check on the local host. Some thing will break for sure. We use new at
many places I suppose.

On Wed, Jan 9, 2019, 2:19 PM Pawan Shukla <[email protected] wrote:

I'm going to post a gif while working on the localhost , will that do ?
thanks!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/plots2/issues/4456#issuecomment-452617760,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AUACQzF54laGmENyIQX3Xljtipq6nPoPks5vBa0HgaJpZM4Zktm-
.

Can you please use grep command for it?
Also, please open up a PR so that I can help you in code.

I've searched for new in the app directory and still didn't find anything that breaks , I'm uploading the gif for and the creating PR ! Thanks for reviewing !

gif for sign up :
user_session

gif for login :
user_session1

Try routes "/login" and "/signup", I think they use new.html.erb. Thanks!

What about the edit profile page, login page, signup page?

On Wed, Jan 9, 2019, 3:13 PM Pawan Shukla <[email protected] wrote:

I've searched for new in the app directory and still didn't find anything
that breaks , I'm uploading the gif for and the creating PR ! Thanks for
reviewing !

gif for sign up :
[image: user_session]
https://user-images.githubusercontent.com/36789080/50890792-e67d6f80-1420-11e9-94c9-32f9b4fd26d6.gif

gif for login :
[image: user_session1]
https://user-images.githubusercontent.com/36789080/50890826-f5fcb880-1420-11e9-81ce-57f270d76942.gif

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/plots2/issues/4456#issuecomment-452634048,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AUACQ1rF5VBUL7ahwgqSOd_d7KtqLuJ8ks5vBbnCgaJpZM4Zktm-
.

@SidharthBansal its in the gifs

The titles were actually mentioning how I created that user session

There needs to be 5 gifs

  1. Login modal with back group anything apart from login,signup page and
    research page
  2. Signup modal with back group anything apart from login,signup page and
    research page
  3. Login page
  4. Signup page
  5. Edit profile page.

On Wed, Jan 9, 2019, 3:23 PM Pawan Shukla <[email protected] wrote:

The titles were actually mentioning how I created that user session

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/plots2/issues/4456#issuecomment-452637150,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AUACQ6d-8eTjf0EKJNAw0UYyY7LWz6Jrks5vBbwWgaJpZM4Zktm-
.

But you provided 2 gifs. 3 gifs are missing.

On Wed, Jan 9, 2019, 3:25 PM Sidharth Bansal [email protected]
wrote:

There needs to be 5 gifs

  1. Login modal with back group anything apart from login,signup page and
    research page
  2. Signup modal with back group anything apart from login,signup page and
    research page
  3. Login page
  4. Signup page
  5. Edit profile page.

On Wed, Jan 9, 2019, 3:23 PM Pawan Shukla <[email protected] wrote:

The titles were actually mentioning how I created that user session

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/plots2/issues/4456#issuecomment-452637150,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AUACQ6d-8eTjf0EKJNAw0UYyY7LWz6Jrks5vBbwWgaJpZM4Zktm-
.

Oh sorry , my bad ! I 'll do it now !

  1. login page :
    login_user-session

4.sign in page:
sign_in_usersession

5.Edit prof:
edit_prof_user-session
Thanks!

Please link the PR. I will check the code.
Thanks for your work.

On Wed, Jan 9, 2019, 3:43 PM Pawan Shukla <[email protected] wrote:

>

  1. login page :
    [image: login_user-session]
    https://user-images.githubusercontent.com/36789080/50892675-162e7680-1425-11e9-8843-ca1ce0364937.gif

4.sign in page:
[image: sign_in_usersession]
https://user-images.githubusercontent.com/36789080/50892697-221a3880-1425-11e9-98c3-fad78ba153a6.gif

5.Edit prof:
[image: edit_prof_user-session]
https://user-images.githubusercontent.com/36789080/50892715-27778300-1425-11e9-870e-b4ada0c0ab85.gif
Thanks!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/plots2/issues/4456#issuecomment-452643223,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AUACQ54zvp7oDrQZIyfjRygFWQVIrbb-ks5vBcC_gaJpZM4Zktm-
.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

noi5e picture noi5e  Â·  3Comments

jywarren picture jywarren  Â·  3Comments

first-timers[bot] picture first-timers[bot]  Â·  3Comments

milaaraujo picture milaaraujo  Â·  3Comments

grvsachdeva picture grvsachdeva  Â·  3Comments