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

Show number of unanswered questions in title of question page #2302

Merged
merged 4 commits into from
Oct 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
49 changes: 49 additions & 0 deletions app/assets/javascripts/favicon.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/**
* Manager for the favicon.
*/
export class FaviconManager {
private readonly tags: Set<string> = new Set<string>();

public constructor(initial: string[]) {
initial.forEach(e => this.tags.add(e));
}

/**
* Request that you want to show a dot in the favicon. This will always
* result in a dot being shown.
*
* @param {string} tag The tag you want to add.
*/
public requestDot(tag: string): void {
// Thread safe, since JS is single threaded in the browser.
this.tags.add(tag);
FaviconManager.showDot();
}

/**
* Indicate you no longer want to show a dot in the favicon. If you were the
* last one requesting the dot, the dot will be hidden.
*
* @param {string} tag The tag you want to release.
*/
public releaseDot(tag: string): void {
this.tags.delete(tag);
if (this.tags.size === 0) {
FaviconManager.hideDot();
}
}

private static showDot(): void {
document.querySelector("link[rel=\"shortcut icon\"][href=\"/icon.png\"]")
?.setAttribute("href", "/icon-not.png");
document.querySelector("link[rel=\"shortcut icon\"][href=\"/favicon.ico\"]")
?.setAttribute("href", "/favicon-not.ico");
}

private static hideDot(): void {
document.querySelector("link[rel=\"shortcut icon\"][href=\"/icon-not.png\"]")
?.setAttribute("href", "/icon.png");
document.querySelector("link[rel=\"shortcut icon\"][href=\"/favicon-not.ico\"]")
?.setAttribute("href", "/favicon.ico");
}
}
11 changes: 6 additions & 5 deletions app/assets/javascripts/notification.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { FaviconManager } from "favicon";
import { fetch } from "util.js";
/**
* Model for a notification in the navbar. It adds three listeners to the notification view:
Expand All @@ -15,13 +16,15 @@ export class Notification {
private readonly element: Element;
private readonly url: string;
private readonly notifiableUrl: string;
private readonly faviconManager: FaviconManager;
private read: boolean;

constructor(id: number, url: string, read: boolean, notifiableUrl: string, installClickHandler: boolean) {
constructor(id: number, url: string, read: boolean, notifiableUrl: string, installClickHandler: boolean, manager: FaviconManager) {
this.element = document.querySelector(`.notification[data-id="${id}"]`);
this.read = read;
this.url = url;
this.notifiableUrl = notifiableUrl;
this.faviconManager = manager;

this.element.querySelector(".read-toggle-button").addEventListener("click", event => {
this.toggleRead();
Expand Down Expand Up @@ -63,12 +66,10 @@ export class Notification {
}
if (document.querySelectorAll(".notification.unread").length === 0) {
document.querySelector("#navbar-notifications .dropdown-toggle")?.classList?.remove("notification");
document.querySelector("link[rel=\"shortcut icon\"][href=\"/icon-not.png\"]")?.setAttribute("href", "/icon.png");
document.querySelector("link[rel=\"shortcut icon\"][href=\"/favicon-not.ico\"]")?.setAttribute("href", "/favicon.ico");
this.faviconManager.releaseDot("notifications");
} else {
document.querySelector("#navbar-notifications .dropdown-toggle")?.classList?.add("notification");
document.querySelector("link[rel=\"shortcut icon\"][href=\"/icon.png\"]")?.setAttribute("href", "/icon-not.png");
document.querySelector("link[rel=\"shortcut icon\"][href=\"/favicon.ico\"]")?.setAttribute("href", "/favicon-not.ico");
this.faviconManager.requestDot("notifications");
}
}

Expand Down
12 changes: 11 additions & 1 deletion app/assets/javascripts/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,15 @@ function makeVisible(element) {
element.style.visibility = "visible";
}

/**
* Set the title of the webpage.
*
* @param {string} title The new title.
*/
function setDocumentTitle(title) {
document.title = title;
}

export {
delay,
fetch,
Expand All @@ -224,5 +233,6 @@ export {
initTooltips,
initTokenClickables,
makeInvisible,
makeVisible
makeVisible,
setDocumentTitle
};
3 changes: 3 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -186,5 +186,8 @@ def set_notifications
true
end
end
# This variable counts for which services the dot in the favicon should be shown.
# On most pages this will be empty or contain :notifications
@dot_icon = @unread_notifications.any? ? %i[notifications] : []
end
end
8 changes: 7 additions & 1 deletion app/controllers/courses_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,6 @@ def scoresheet
end

def questions
@title = I18n.t('courses.questions.questions.title')
@crumbs = [[@course.name, course_path(@course)], [I18n.t('courses.questions.questions.title'), '#']]

@refresh = ActiveRecord::Type::Boolean.new.deserialize(params.fetch(:refresh, @course.enabled_questions?.to_s))
Expand All @@ -212,6 +211,13 @@ def questions
.order(updated_at: :desc)
.includes(:user, :last_updated_by, submission: [:exercise])
.paginate(page: parse_pagination_param(params[:answered_page]), per_page: 10)
count = @course.unanswered_questions.count
@title = if count == 0
I18n.t('courses.questions.questions.title')
else
I18n.t('courses.questions.questions.page_title', count: count)
end
@dot_icon.push(:questions) if @unanswered.any?
end

def update_membership
Expand Down
2 changes: 2 additions & 0 deletions app/javascript/packs/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { Toast } from "toast";
import { Notification } from "notification";
import { checkTimeZone, checkIframe, initCSRF, initTooltips } from "util.js";
import { initClipboard } from "copy";
import { FaviconManager } from "favicon";

// Initialize clipboard.js
initClipboard();
Expand All @@ -42,6 +43,7 @@ $(initTooltips);

// Use a global dodona object to prevent polluting the global na
const dodona = window.dodona || {};
dodona.dotManager = new FaviconManager(dodona.dotCount || []);
dodona.checkTimeZone = checkTimeZone;
dodona.Toast = Toast;
dodona.Notification = Notification;
Expand Down
2 changes: 2 additions & 0 deletions app/javascript/packs/course.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
import {
QuestionTable
} from "question_table.ts";
import { setDocumentTitle } from "util.js";

window.dodona.initSeriesReorder = initSeriesReorder;
window.dodona.initCourseForm = initCourseForm;
Expand All @@ -19,3 +20,4 @@ window.dodona.initCourseMembers = initCourseMembers;
window.dodona.loadUsers = loadUsers;

window.dodona.questionTable = QuestionTable;
window.dodona.setDocumentTitle = setDocumentTitle;
6 changes: 6 additions & 0 deletions app/views/courses/questions.js.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,9 @@ document.querySelector(".question-table-answered").innerHTML = "<%= escape_javas
<% else %>
document.getElementById("questions-in-progress").classList.remove("hidden");
<% end %>
dodona.setDocumentTitle("<%= @title %> - Dodona");
<% if @unanswered.any? %>
dodona.dotManager.requestDot("questions");
<% else %>
dodona.dotManager.releaseDot("questions");
<% end %>
6 changes: 4 additions & 2 deletions app/views/layouts/application.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
<meta name="msapplication-TileColor" content="#2196F3"/>
<meta name="msapplication-TileImage" content="/icon.png"/>
<link rel="apple-touch-icon-precomposed" href="/icon.png"/>
<link rel="shortcut icon" sizes="192x192" href="<%= @unread_notifications&.any? ? "/icon-not.png" : "/icon.png" %>">
<link rel="shortcut icon" href="<%= @unread_notifications&.any? ? "/favicon-not.ico" : "/favicon.ico" %>">
<link rel="shortcut icon" sizes="192x192" href="<%= @dot_icon&.any? ? "/icon-not.png" : "/icon.png" %>">
<link rel="shortcut icon" href="<%= @dot_icon&.any? ? "/favicon-not.ico" : "/favicon.ico" %>">
<link rel="manifest" href="/manifest.json">
<title><%= "#{@title} - " if @title %>Dodona</title>
<link href="https://fonts.googleapis.com/css2?family=Roboto:ital,wght@0,300;0,400;0,500;0,700;1,400&display=swap" rel="stylesheet">
Expand All @@ -29,6 +29,7 @@
<script>
window.dodona = window.dodona || {};
window.dodona.darkMode = false;
window.dodona.dotCount = <%= @dot_icon.to_json.html_safe %>;
const lightModeCSS = '<%= stylesheet_link_tag "application", media: "all" %>';
const darkModeCSS = '<%= stylesheet_link_tag "application-dark", media: "all" %>';

Expand Down Expand Up @@ -63,6 +64,7 @@
<script>
window.dodona = window.dodona || {};
window.dodona.darkMode = <%= session.include?(:dark) && session[:dark] %>;
window.dodona.dotCount = <%= @dot_icon&.to_json&.html_safe || "[]" %>;
</script>
<% end %>

Expand Down
2 changes: 1 addition & 1 deletion app/views/notifications/_notifications_table.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
<script>
$(() => {
<% notifications.each do |n| %>
new dodona.Notification(<%= n.id %>, "<%= notification_path(n, format: :json) %>", <%= n.read %>, "<%= notifiable_url(n) %>", false);
new dodona.Notification(<%= n.id %>, "<%= notification_path(n, format: :json) %>", <%= n.read %>, "<%= notifiable_url(n) %>", false, dodona.dotManager);
<% end %>
})
</script>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
<script>
$(() => {
<% notifications.each do |n| %>
new dodona.Notification(<%= n.id %>, "<%= notification_path(n, format: :json) %>", <%= n.read %>, "<%= notifiable_url(n) %>", true);
new dodona.Notification(<%= n.id %>, "<%= notification_path(n, format: :json) %>", <%= n.read %>, "<%= notifiable_url(n) %>", true, dodona.dotManager);
<% end %>
})
</script>
Expand Down
1 change: 1 addition & 0 deletions config/locales/views/courses/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ en:
open: "Unanswered questions"
in_progress: "Questions in progress"
closed: "Answered questions"
page_title: "(%{count}) Questions"
title: "Questions"
auto_refresh: Automatically refresh questions
ago: "%{when} ago"
Expand Down
1 change: 1 addition & 0 deletions config/locales/views/courses/nl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ nl:
in_progress: "Vragen in behandeling"
closed: "Beantwoorde vragen"
title: "Vragen"
page_title: "(%{count}) Vragen"
auto_refresh: Vragen automatisch vernieuwen
ago: "%{when} geleden"
last_edited_by:
Expand Down
13 changes: 13 additions & 0 deletions test/controllers/courses_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -571,4 +571,17 @@ def with_users_signed_in(users)
assert :ok, "#{who} should not be able to view questions"
end
end

test 'question page title is correct' do
sign_in @admins.first
get questions_course_path(@course)
assert_select 'title', /^([^0-9]*)$/

submission = create :submission, course: @course
create :question, question_state: :answered, submission: submission
create :question, question_state: :unanswered, submission: submission
create :question, question_state: :in_progress, submission: submission
get questions_course_path(@course)
assert_select 'title', /\(1\)/
end
end