Skip to content

Show "Last login" label next to the login method #622

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.

Sign up for GitHub

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Show "Last login" label next to the login method #622

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
2042860
Show "Last login" label next to the login method
humitos Jul 2, 2025
0382b81
Automatically switch to Email tab when last method was email
humitos Jul 8, 2025
05e535e
Use a tooltip
humitos Jul 14, 2025
6c0da56
Merge branch 'main' of github.com:readthedocs/ext-theme into humitos/…
humitos Jul 14, 2025
2f839e2
Handle `last_login_method` and `login_tab`
humitos Jul 14, 2025
af7c6e6
Rename
humitos Jul 14, 2025
a089234
Tooltip position
humitos Jul 14, 2025
5ea2c4e
Bring back the beta label
humitos Jul 14, 2025
b57e447
Handle click on buttons to save the cookie
humitos Jul 14, 2025
8a5b4fa
Small linting issues
humitos Jul 14, 2025
688119a
Always show it at the right
humitos Jul 14, 2025
5bece7f
Improve code that removes the `visible` property
humitos Jul 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions readthedocsext/theme/static/readthedocsext/theme/js/site.js
View file
Open in desktop

Large diffs are not rendered by default.

31 changes: 26 additions & 5 deletions readthedocsext/theme/templates/account/login.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@
</div>
</div>

<div class="ui small stackable teal centered fluid wrapping menu"
data-bind="semanticui: {tabs: {history: true, autoTabActivation: false}}">
<div class="ui small stackable teal centered fluid wrapping menu" data-bind="semanticui: {tabs: {
{% if last_login_tab %}autoTabActivation: '{{ last_login_tab }}'{% endif %}
}}">

<a class="item" data-tab="vcs">
<i class="fas fa-cloud icon"></i>
Expand All @@ -42,6 +43,7 @@

{# If allowed providers is given, disable the email menu item #}
<a class="{% if allowed_providers %}disabled{% endif %} item"
{% if last_login_tab == "email" %} data-tooltip="Last used" data-variation="teal visible" data-position="top center" {% endif %}
data-tab="email">
<i class="fas fa-envelope icon"></i>
{% block authentication_email_text %}
Expand All @@ -50,8 +52,7 @@
</a>

{% if USE_ORGANIZATIONS %}
<a class="item"
href="{% url "saml_resolve_login" %}{% if redirect_field_value %}?{{ redirect_field_name }}={{ redirect_field_value }}{% endif %}">
<a class="item" data-tab="sso">
<i class="fas fa-shield-alt icon"></i>
{% trans "Single sign-on" %}
</a>
Expand Down Expand Up @@ -100,14 +101,34 @@
<div class="ui stackable relaxed text menu">
<a class="item" href="{% url 'account_reset_password' %}">{% trans "Forgot your password?" %}</a>
<div class="right menu">
<button class="ui primary button" type="submit">{% trans "Log in" %}</button>
<button class="ui primary button"
data-bind="click: $root.save_login_method"
Copy link
Contributor

Choose a reason for hiding this comment

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

You shouldn't use $root here, this method should be in a dedicated view class. $root is currently only used for some utility methods that are shared across many views.

data-provider="email"
type="submit">{% trans "Log in" %}</button>
</div>
</div>

</form>
{% endblock authentication_email %}
</div>

{% if USE_ORGANIZATIONS %}
<div class="ui basic center aligned tab segment" data-tab="sso">
<a href="{% url "saml_resolve_login" %}{% if redirect_field_value %}?{{ redirect_field_name }}={{ redirect_field_value }}{% endif %}">

<button class="ui button"
data-bind="click: $root.save_login_method"
Copy link
Contributor

Choose a reason for hiding this comment

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

Another mention of $root, should be a method in a view class.

data-provider="sso"
type="submit"
{% if last_login_method == "sso" %} data-tooltip="Last used" data-variation="teal visible" data-position="right center" {% endif %}
title="Single sign-on">
<i class="fas fa-shield-alt icon" aria-hidden="true"></i>
{% trans "Log in using single sign-on" %}
</button>
</a>
</div>
{% endif %}

{% block authentication_extra %}
{% endblock authentication_extra %}

Expand Down
1 change: 1 addition & 0 deletions readthedocsext/theme/templates/socialaccount/snippets/provider_list.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
{% trans "GitHub" as provider_name %}
<li class="item">
<a class="ui button"
{% if last_login_method == "github" or last_login_method == "githubapp" %} data-tooltip="Last used" data-variation="teal visible" data-position="right center" {% endif %}
data-bind="click: $root.show_modal('github-select')"
title="{{ provider_name }}">
<i class="fa-brands fa-github icon"></i>
Expand Down
26 changes: 19 additions & 7 deletions readthedocsext/theme/templates/socialaccount/snippets/provider_list_item.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,26 @@
action="{% provider_login_url provider.id process=process scope=scope auth_params=auth_params %}">
{% csrf_token %}

{# djlint: off #}
Copy link
Contributor

Choose a reason for hiding this comment

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

You should use something less generic than off. If there is a rule you're trying to avoid, limit it to that rule.

<button class="ui {{ button_classes }} button"
data-bind="click: $root.save_login_method"
Copy link
Contributor

Choose a reason for hiding this comment

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

Another $root

data-provider="{{ provider.id }}"
type="submit"
title="{{ provider.name }}">
<i class="fa-brands fa-{{ provider.name|lower }} icon"></i>
{% blocktrans trimmed with provider_name=provider.app.name|default:provider.name verbiage=verbiage|default:'Connect to' %}
{{ verbiage }} {{ provider_name }}
{% endblocktrans %}
</button>

</form>
{% if last_login_method == provider.id|lower %}
data-tooltip="Last used"
data-variation="teal visible"
data-position="right center"
{% endif %}

title="{{ provider.name }}">
{# djlint: on #}

<i class="fa-brands fa-{{ provider.name|lower }} icon"></i>
{% blocktrans trimmed with provider_name=provider.app.name|default:provider.name verbiage=verbiage|default:'Connect to' %}
{{ verbiage }} {{ provider_name }}
{% endblocktrans %}
</button>

</form>
{% endif %}
36 changes: 36 additions & 0 deletions src/js/application/views.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,16 @@ export class ApplicationView {
if (found_modal.length === 0) {
console.debug("Modal not found:", selector);
}

// Remove "visible" variation property to remove "Last used" tooltip
const buttons = document.querySelectorAll("button.ui");
for (const button of buttons) {
if (button.dataset.variation) {
const variation = button.dataset.variation.split(" ");
variation.pop("visible");
button.dataset.variation = variation.join(" ");
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

You should always use KO view code to manipulate the DOM. Using JS queries or jQuery to manipulate the DOM in addition is mixing patterns and should be always avoided.

I'm a bit confused what this is trying to do though. What is the intention here? It also seems like this would affect all modals, which we don't want.

};
}

Expand Down Expand Up @@ -94,4 +104,30 @@ export class ApplicationView {
}
return false;
}

/**
* Save the provider used for login.
*
* This could be used like:
*
* .. code:: html
*
* <form method="post" action="...">
* <button data-bind="click: $root.save_login_method">
Copy link
Contributor

Choose a reason for hiding this comment

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

Also $root here.

* Log in using GitHub
* </button>
* </form>
*
* @param {Object} data - Context data
* @param {Event} event - Click event
* @returns {knockout_click}
*/
save_login_method(data, event) {
const elem = event.currentTarget;
if (window.isSecureContext) {
console.debug("Setting last login method: ", elem.dataset.provider);
cookieStore.set("last-login-method", elem.dataset.provider);
}
return true;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be implemented on a dedicated view class, not on the main application view. The main application view is strictly a router to these dedicated views and a couple utility methods.

You should look at any of the other templates that use a model view and work from those templates, they all follow the same pattern with a dedicated view class for each view.

}