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

Submit language form on language link click #71

Merged
merged 1 commit into from
Jul 26, 2018

Conversation

jraddaoui
Copy link
Collaborator

@jraddaoui jraddaoui commented Jul 26, 2018

To avoid having a submit button inside the language drop-down to send
the language change request, create a hidden field inside the form for
the language and remove the current check-boxes. Add drop-down links
outside the form and change the language input value on link click and
submit the form.

Connects to #16.

@jraddaoui jraddaoui added this to the beta milestone Jul 26, 2018
@jraddaoui jraddaoui self-assigned this Jul 26, 2018
@jraddaoui jraddaoui requested a review from sevein July 26, 2018 14:38
@codecov
Copy link

codecov bot commented Jul 26, 2018

Codecov Report

Merging #71 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master     #71   +/-   ##
======================================
  Coverage    89.3%   89.3%           
======================================
  Files          41      41           
  Lines        1506    1506           
======================================
  Hits         1345    1345           
  Misses        161     161

{% get_available_languages as LANGUAGES %}
{% get_language_info_list for LANGUAGES as languages %}
{% for language in languages %}
<a class="dropdown-item" href="javascript:;" onclick="document.langForm.language.value='{{ language.code }}'; document.langForm.submit();">{{ language.name_local|capfirst }} ({{ language.code }})</a>
Copy link
Collaborator

Choose a reason for hiding this comment

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

You came this far without a single .js file but perhaps you should have one now to avoid mixing code and markup. It used to be a bad practice IIRC.

@jraddaoui
Copy link
Collaborator Author

Thanks @sevein! Please, could you take another look?

PS: I've created #72 to check JS code in the CI process.

<script type="text/javascript" src="{% static 'js/jquery-3.2.1.min.js' %}"></script>
<script type="text/javascript" src="{% static 'js/popper.min.js' %}"></script>
<script type="text/javascript" src="{% static 'js/bootstrap.min.js' %}"></script>
<script type="text/javascript" src="{% static 'js/custom.js' %}"></script>
Copy link
Collaborator

Choose a reason for hiding this comment

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

FWIW HTML 5's type defaults to text/javascript. It's fine to be explicit but just for your information.

Copy link
Collaborator

@sevein sevein left a comment

Choose a reason for hiding this comment

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

Nicely done!

To avoid having a submit button inside the language drop-down to send
the language change request, create a hidden field inside the form for
the language and remove the current check-boxes. Add drop-down links
outside the form and change the language input value on link click and
submit the form.

Squashed commit of the following:

commit 6a84f23
Author: José Raddaoui Marín <raddaouimarin@gmail.com>
Date:   Thu Jul 26 20:23:17 2018 +0200

    Move form modification and submission to JS file

commit 3103c50
Author: José Raddaoui Marín <raddaouimarin@gmail.com>
Date:   Thu Jul 26 16:35:30 2018 +0200

    Submit language form on language link click

    To avoid having a submit button inside the language drop-down to send
    the language change request, create a hidden field inside the form for
    the language and remove the current check-boxes. Add drop-down links
    outside the form and change the language input value on link click and
    submit the form. Include the JS code used for this behavior in the link
    attributes to avoid loading an entire custom JS only for this.
@jraddaoui jraddaoui merged commit 333f012 into master Jul 26, 2018
@jraddaoui jraddaoui removed the review label Jul 26, 2018
@jraddaoui jraddaoui deleted the dev/issue-16-lang-form branch July 26, 2018 19:04
@sallain sallain added this to Done in SCOPE Mar 26, 2019
@sallain sallain removed this from Done in SCOPE Mar 26, 2019
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.

None yet

2 participants