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

Separate edit profile page #157

Merged
merged 16 commits into from
Feb 5, 2021
Merged

Separate edit profile page #157

merged 16 commits into from
Feb 5, 2021

Conversation

LinqLover
Copy link
Contributor

@LinqLover LinqLover commented Jan 29, 2021

Part of #47, see #47 (comment).

Tasks:

  • New view/route (edit_profile) to change name, email address, and place from same controller (extract from user edit + account settings)
  • Link to it from edit user page
  • Remove email address from account settings view

In particular note the monster commit cf9060f and the relevant commit message ...

@LinqLover LinqLover self-assigned this Jan 29, 2021
Surprisingly, all the devise views saved in app/views/users were not actually used by devise because in devise.rb, config.scoped_views needed to be enabled first. After enabling it, it turned out that the users views were not configured with devise-bootstrap-views until now. So I regenerated them and as another consequence, it turned out that their corresponding i18n data were missing, too, so I also added devise-i18n-bootstrap to the Gemfile. Sigh ...
@LinqLover LinqLover marked this pull request as ready for review January 29, 2021 17:21
@Paula-Kli
Copy link
Contributor

what is the difference between /edit and /edit_profile? is there a particular reason why they are not the same page?

@LinqLover
Copy link
Contributor Author

what is the difference between /edit and /edit_profile? is there a particular reason why they are not the same page?

Yes, this is a part of #47, where we want to have a settings/profile page with an edit link which then leads you to the edit_profile page. Design will be worked out in another PR, but in the end the edit page will show up a modal dialog with the edit_profile form when requested.

Copy link
Contributor

@felixauringer felixauringer left a comment

Choose a reason for hiding this comment

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

Nice! I always wondered how to customize devise views 😅

I find this PR really hard to review. Would it also be possible to customize only some views of devise?

app/views/users/edit.html.erb Show resolved Hide resolved
app/views/users/mailer/confirmation_instructions.html.erb Outdated Show resolved Hide resolved
app/views/users/shared/_links.erb Outdated Show resolved Hide resolved
config/locales/devise.en.yml Outdated Show resolved Hide resolved
Copy link
Collaborator

@rheaSPK rheaSPK left a comment

Choose a reason for hiding this comment

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

Looks fine to me (except some minor things @felixauringer mentioned), thanks for the reconfiguration, must have been annoying ^^

@LinqLover
Copy link
Contributor Author

@felixauringer Thanks for your review! All your suggestions but the first one refer to auto-generated code (I did not write it by hand ...). Do you think it is a good idea to change these auto-generations? Otherwise, I'd just leave it as-is.

@LinqLover
Copy link
Contributor Author

I find this PR really hard to review. Would it also be possible to customize only some views of devise?

Hard for the same reason because then we would mix up the old and the new style and could not rely on the autogeneration any longer. But I promise you that I really did not make any manual changes in cf9060f :-)

@felixauringer
Copy link
Contributor

@felixauringer Thanks for your review! All your suggestions but the first one refer to auto-generated code (I did not write it by hand ...). Do you think it is a good idea to change these auto-generations? Otherwise, I'd just leave it as-is.

Fine for me, only some red tests to work on 👍

@LinqLover LinqLover merged commit 00cce3b into dev Feb 5, 2021
@LinqLover LinqLover deleted the extract-edit-profile branch February 5, 2021 15:25
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.

4 participants