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

New accounts #1193

Closed
wants to merge 8 commits into from
Closed

Conversation

AbhijithGanesh
Copy link
Collaborator

@AbhijithGanesh AbhijithGanesh commented Nov 8, 2021

Closes #1199

Description

replaces the update-settings.html and removes settings.html, mnimalistic html changes.

@brylie
Copy link
Member

brylie commented Nov 8, 2021

I'm converting this to a draft while it is work-in-progress.

@brylie brylie marked this pull request as draft November 8, 2021 10:52
@AbhijithGanesh
Copy link
Collaborator Author

@gorkemarslan What should I change, I am a bit confused actually!😅

@gorkemarslan
Copy link
Collaborator

gorkemarslan commented Nov 14, 2021

@AbhijithGanesh, why did you revert your initial changes? 😅

@brylie wants you to create new views. I guess this is one of the missing parts.

Edit: I didn't completely review your first changes but they seem ok with me. At which point do you need clarification? Are you confused by the template part? Apparently, you did well it. Again, I didn't try to run your changes.

Of course, we are going to think of views part together and find a way to solve that.

@AbhijithGanesh
Copy link
Collaborator Author

My doubt is, are we going to make new views for Accounts ? I have restructured the templates I reckon.

@brylie
Copy link
Member

brylie commented Nov 15, 2021

@brylie wants you to create new views. I guess this is one of the missing parts.

New views are optional. The wording in the issue is "as needed," meaning we may not need them.

The goal is to have a fully functional accounts app with only Django templates and views to remove the Backbone code. Implementation details are at your discretion.

@AbhijithGanesh
Copy link
Collaborator Author

This PR has a fully functional accounts app right? I've removed the backbone code too I guess. If I haven't please request a review, I will make changes.

@AbhijithGanesh AbhijithGanesh marked this pull request as ready for review November 15, 2021 17:33
@brylie
Copy link
Member

brylie commented Nov 15, 2021

@AbhijithGanesh, I've created a new issue to list all of the code changes needed for account.html:

#1199

Would you like to focus just on the single account.html file?

@AbhijithGanesh
Copy link
Collaborator Author

Yes I shall take note of this and focus on a single file

Copy link
Member

@brylie brylie left a comment

Choose a reason for hiding this comment

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

[deleted]

@@ -8,7 +8,6 @@
<link type="text/css" rel="stylesheet/less" href="{% static "less/setup.less" %}"/>
{% endblock extra_css %}

{% block backbone_template %}
Copy link
Member

Choose a reason for hiding this comment

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

Does the user setup wizard still work with this block tag removed?

@AbhijithGanesh
Copy link
Collaborator Author

@brylie Closing this issue since It has been distributed.

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.

port account.html to Django template syntax
3 participants