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

feat: add cancel button to logout #59

Merged
merged 5 commits into from
Apr 7, 2025

Conversation

samikshakhadka
Copy link
Contributor

fix: #57

Copy link
Contributor

@Mr-Sunglasses Mr-Sunglasses left a comment

Choose a reason for hiding this comment

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

{% url 'account_login' as login_url %}
    {% element form method="get" action=login_url %}
        {% slot actions %}
            {% element button type="submit" %}
                {% trans 'Cancel' %}
            {% endelement %}
        {% endslot %}
    {% endelement %}

This is one of the pythonic way I found to do this. :-)

{% endslot %}
{% endelement %}
<script>
document.getElementById('cancel-button').addEventListener('click', function() {
history.back();
Copy link
Contributor

@Mr-Sunglasses Mr-Sunglasses Mar 31, 2025

Choose a reason for hiding this comment

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

using history.back() may be unpredictable sometimes, I think it is better to do it in a pythonic way.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I agree with not using Javascript for this.

@Mariatta
Copy link
Member

Another option is to simply log people out without the confirmation step.

@samikshakhadka
Copy link
Contributor Author

Thanks for the insights!

@Mariatta I agree removing the confirmation step simplifies the flow.
@Mr-Sunglasses Your Pythonic approach is a clean solution if we keep the confirmation.

Proposal: Let’s proceed with removing the confirmation (per @Mariatta’s suggestion) since it reduces complexity. We can revisit the cancel button pattern if needed later.

Unless there are objections, I’ll update the PR shortly.

@samikshakhadka
Copy link
Contributor Author

I've added the ACCOUNT_LOGOUT_ON_GET = True setting to the AllAuth configuration to enable direct user logout without requiring a confirmation page.

Since JavaScript isn't currently being used for this functionality, I thought this settings-based approach might be the simplest solution while maintaining compatibility with the rest of the system.

If JavaScript becomes an option in the future, we could consider implementing an approach that converts a logout link click into a POST request while still avoiding the confirmation page.

Please let me know if there's a different approach you'd prefer for handling this requirement.

Copy link
Contributor

@Mr-Sunglasses Mr-Sunglasses left a comment

Choose a reason for hiding this comment

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

  • Please remove __pycache__ folder.

To remove pycache file now after commit please reference to this stackoverflow link: https://stackoverflow.com/a/71930352/16563091

  • Just one change, and we are good to go.

@samikshakhadka
Copy link
Contributor Author

  • Please remove __pycache__ folder.

To remove pycache file now after commit please reference to this stackoverflow link: https://stackoverflow.com/a/71930352/16563091

  • Just one change, and we are good to go.

Removed pycache and added it to .gitignore. Thanks for the suggestion @Mr-Sunglasses

Copy link
Member

@Mariatta Mariatta left a comment

Choose a reason for hiding this comment

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

Thanks!

@Mariatta Mariatta merged commit c564fe7 into pyladies:main Apr 7, 2025
1 check passed
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.

Add Cancel button to logout confirmation dialog
3 participants