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

Change "body" to "main" in entrance.html #5410

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

theptrk
Copy link
Contributor

@theptrk theptrk commented Sep 29, 2024

Description

The current setup for account sign up and signin loses the nav bar from the base.html
I am proposing we utilize the block main to regain the nav bar.

Checklist:

  • I've made sure that tests are updated accordingly (especially if adding or updating a template option)

The tests fail for me using main branch before this change, not sure its related

file /Users/patricktran/projamming/tmp/cookiecutter-django/tests/test_cookiecutter_generation.py, line 380
  @pytest.mark.parametrize(
      ["editor", "pycharm_docs_exist"],
      [
          ("None", False),
          ("PyCharm", True),
          ("VS Code", False),
      ],
  )
  def test_pycharm_docs_removed(cookies, context, editor, pycharm_docs_exist):
E       fixture 'cookies' not found
>       available fixtures: _dj_autoclear_mailbox, _django_clear_site_cache, _django_db_helper, _django_db_marker, _django_set_urlconf, _django_setup_unittest, _fail_for_invalid_template_variable, _live_server_helper, _session_faker, _template_string_if_invalid_marker, admin_client, admin_user, anyio_backend, anyio_backend_name, anyio_backend_options, async_client, async_rf, cache, capfd, capfdbinary, caplog, capsys, capsysbinary, client, context, db, django_assert_max_num_queries, django_assert_num_queries, django_capture_on_commit_callbacks, django_db_blocker, django_db_createdb, django_db_keepdb, django_db_modify_db_settings, django_db_modify_db_settings_parallel_suffix, django_db_modify_db_settings_tox_suffix, django_db_modify_db_settings_xdist_suffix, django_db_reset_sequences, django_db_serialized_rollback, django_db_setup, django_db_use_migrations, django_mail_dnsname, django_mail_patch_dns, django_test_environment, django_user_model, django_username_field, doctest_namespace, faker, live_server, mailoutbox, monkeypatch, pytestconfig, record_property, record_testsuite_property, record_xml_attribute, recwarn, rf, settings, tmp_path, tmp_path_factory, tmpdir, tmpdir_factory, transactional_db
>       use 'pytest --fixtures [testpath]' for help on them.
  • I've updated the documentation or confirm that my change doesn't require any updates (I do not see this in docs)

Rationale

Seems like the original purpose of the main block was to allow this.

entrance.html already has a block content inside the html. Presumably to act as a base template for the allauth views.
Screenshot 2024-09-29 at 11 39 37 AM

since our base.html relies on a block content, we now have duplicate names.
Screenshot 2024-09-29 at 11 40 46 AM

In order to get around this issue of duplicate names, base.html block content is wrapped with block main.
This way another template using a block content can still extend base.html and keep the functionality of the nav bar, css, javascript and other behavior.

Without this change (default), the sign up and sign in screens have no way to navigate back to the main page.

Screenshot 2024-09-29 at 11 43 43 AM

@foarsitter
Copy link
Collaborator

Thank you for your pull-request!

I'm missing the reason behind your change: why do you want to display the nav-bar on the login page?

Don't have any numbers but almost everyone seems to isolate the login-form on page nowadays.

The reason why I left the nav-bar out of the page is that allauth, the underlying package we are using, does the same and I want to stick as much as possible to the original source.

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.

2 participants