Skip to content

Commit

Permalink
Merge pull request #2302 from dodona-edu/feature/question-title
Browse files Browse the repository at this point in the history
Show number of unanswered questions in title of question page
  • Loading branch information
niknetniko authored Oct 6, 2020
2 parents d48c6aa + 0b19633 commit 0a80b37
Show file tree
Hide file tree
Showing 14 changed files with 107 additions and 11 deletions.
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

0 comments on commit 0a80b37

Please sign in to comment.