Skip to content

feat: Use dropdown for units when there are more than would fit in the UI #321

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

Closed
xitij2000 wants to merge 2 commits into from
Closed

feat: Use dropdown for units when there are more than would fit in the UI #321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a4df83a
feat: add canvas integration support (#319)
arslanashraf7 Nov 18, 2024
f7da073
feat: Use dropdown for units when there are too many to fit on page
xitij2000 May 19, 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
3 changes: 3 additions & 0 deletions lms/djangoapps/instructor/views/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2516,6 +2516,9 @@ def _list_instructor_tasks(request, course_id, serialize_data=None):
else:
# Specifying for single problem's history
tasks = task_api.get_instructor_task_history(course_id, module_state_key)
elif request.GET.get('include_canvas') is not None:
from ol_openedx_canvas_integration.task_helpers import get_filtered_instructor_tasks # pylint: disable=import-error
tasks = get_filtered_instructor_tasks(course_id, request.user)
else:
# If no problem or student, just get currently running tasks
tasks = task_api.get_running_instructor_tasks(course_id)
Expand Down
6 changes: 6 additions & 0 deletions lms/djangoapps/instructor_task/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ def get_task_completion_info(instructor_task): # lint-amnesty, pylint: disable=

student = None
problem_url = None
entrance_exam_url = None
email_id = None
task_input = None
try:
task_input = json.loads(instructor_task.task_input)
except ValueError:
Expand Down Expand Up @@ -192,6 +194,10 @@ def get_task_completion_info(instructor_task): # lint-amnesty, pylint: disable=
else: # num_succeeded < num_attempted
# Translators: {action} is a past-tense verb that is localized separately. {succeeded} and {attempted} are counts. # lint-amnesty, pylint: disable=line-too-long
msg_format = _("Problem {action} for {succeeded} of {attempted} students")
elif task_input and task_input.get('course_key'):
from ol_openedx_canvas_integration.utils import get_task_output_formatted_message # pylint: disable=import-error
msg_format = get_task_output_formatted_message(task_output)
succeeded = True
elif email_id is not None:
# this reports on actions on bulk emails
if num_attempted == 0:
Expand Down
3 changes: 3 additions & 0 deletions lms/static/js/instructor_dashboard/instructor_dashboard.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,9 @@ such that the value can be defined later than this assignment (file load order).
}, {
constructor: window.InstructorDashboard.sections.ECommerce,
$element: idashContent.find('.' + CSS_IDASH_SECTION + '#e-commerce')
}, {
constructor: window.InstructorDashboard.sections.CanvasIntegration,
$element: idashContent.find('.' + CSS_IDASH_SECTION + '#canvas_integration')
}, {
constructor: window.InstructorDashboard.sections.Membership,
$element: idashContent.find('.' + CSS_IDASH_SECTION + '#membership')
Expand Down
6 changes: 4 additions & 2 deletions lms/static/js/instructor_dashboard/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@
enableColumnReorder: false,
autoHeight: true,
rowHeight: 100,
forceFitColumns: true
forceFitColumns: true,
enableTextSelectionOnCells: true
};
columns = [
{
Expand Down Expand Up @@ -492,7 +493,8 @@
enableCellNavigation: true,
enableColumnReorder: false,
rowHeight: 30,
forceFitColumns: true
forceFitColumns: true,
enableTextSelectionOnCells: true
};
columns = [
{
Expand Down
16 changes: 16 additions & 0 deletions lms/templates/instructor/instructor_dashboard_2/html-datatable.underscore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<p>
<h2><%- title %></h2>
<table class="stat_table"><tr>
<%_.forEach(header, function (h) {%>
<th><%- h %></th>
<%})%>
</tr>
<%_.forEach(data, function (row) {%>
<tr>
<%_.forEach(row, function (value) {%>
<td><%- value %></td>
<%})%>
</tr>
<%})%>
</table>
</p>
2 changes: 1 addition & 1 deletion lms/templates/instructor/instructor_dashboard_2/instructor_dashboard_2.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@

## Include Underscore templates
<%block name="header_extras">
% for template_name in ["cohorts", "discussions", "enrollment-code-lookup-links", "cohort-editor", "cohort-group-header", "cohort-selector", "cohort-form", "notification", "cohort-state", "divided-discussions-inline", "divided-discussions-course-wide", "cohort-discussions-category", "cohort-discussions-subcategory", "certificate-allowlist", "certificate-allowlist-editor", "certificate-bulk-allowlist", "certificate-invalidation", "membership-list-widget"]:
% for template_name in ["cohorts", "discussions", "enrollment-code-lookup-links", "cohort-editor", "cohort-group-header", "cohort-selector", "cohort-form", "notification", "cohort-state", "divided-discussions-inline", "divided-discussions-course-wide", "cohort-discussions-category", "cohort-discussions-subcategory", "certificate-allowlist", "certificate-allowlist-editor", "certificate-bulk-allowlist", "certificate-invalidation", "membership-list-widget", "html-datatable"]:
<script type="text/template" id="${template_name}-tpl">
<%static:include path="instructor/instructor_dashboard_2/${template_name}.underscore" />
</script>
Expand Down
22 changes: 21 additions & 1 deletion lms/templates/seq_block.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,23 @@
from django.conf import settings
%>

<script type="text/template" id="dropdown-button-tpl">
<li id="dropdown-container" class="h-100">
<button
id="dropdown-sequence-list-button"
class="dropdown-toggle"
type="button"
>
<span class="icon fa fa-chevron-down" aria-hidden="true"></span>
</button>
<div id="dropdown-sequence-list" style="display: none; position: absolute; width: 240px; right: 0;">

@gemini-code-assist gemini-code-assist Bot Jun 3, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The div#dropdown-sequence-list uses inline styles. For better maintainability and separation of concerns, could these styles be moved to a CSS class in SequenceBlockDisplay.css? This would also make it easier to manage styles consistently.

<ol class="d-block dropdown-menu bg-white py-0 shadow-sm border"
aria-labelledby="dropdown-sequence-list-button">
</ol>
</div>
</li>
</script>

<div id="sequence_${element_id}" class="sequence" data-id="${item_id}"
data-position="${position}"
data-next-url="${next_url}" data-prev-url="${prev_url}"
Expand Down Expand Up @@ -46,7 +63,7 @@
</li>
% else:
% for idx, item in enumerate(items):
<li role="presentation">
<li role="presentation" class="sequence-list-item">
<button class="seq_${item['type']} inactive nav-item tab"
role="tab"
tabindex="-1"
Expand Down Expand Up @@ -98,6 +115,9 @@
</ul>
</li>
% endif
% if show_dropdown:
## <%include file='seq_dropdown.html' args="items=items[15:], start_index=15"/>
Comment thread
xitij2000 marked this conversation as resolved.
% endif
</ol>
</nav>
</div>
Expand Down
76 changes: 76 additions & 0 deletions xmodule/js/src/sequence/display.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
(function() {
'use strict';

const { HtmlUtils } = window.edx;

this.Sequence = (function() {
function Sequence(element, runtime) {
var self = this;
Expand All @@ -26,6 +28,9 @@
this.goto = function(event) {
return Sequence.prototype.goto.apply(self, [event]);
};
this.toggleDropdown = function(event) {
return Sequence.prototype.toggleDropdown.apply(self, [event]);
};
this.toggleArrows = function() {
return Sequence.prototype.toggleArrows.apply(self);
};
Expand All @@ -38,6 +43,12 @@
this.displayTabTooltip = function(event) {
return Sequence.prototype.displayTabTooltip.apply(self, [event]);
};
this.renderDropdown = function() {
return Sequence.prototype.renderDropdown.apply(self);
}
this.handleClickOutsideDropdown = function(event) {
return Sequence.prototype.handleClickOutsideDropdown.apply(self, [event]);
}
this.arrowKeys = {
LEFT: 37,
UP: 38,
Expand All @@ -62,23 +73,87 @@
this.showCompletion = this.el.data('show-completion');
this.keydownHandler($(element).find('#sequence-list .tab'));
this.base_page_title = ($('title').data('base-title') || '').trim();
this.dropdownButtonTpl = _.template($('#dropdown-button-tpl').text())({});

@gemini-code-assist gemini-code-assist Bot Jun 3, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

The template dropdown-button-tpl uses static IDs (e.g., dropdown-container, dropdown-sequence-list-button, dropdown-sequence-list). If multiple sequence blocks are rendered on the same page, these IDs will clash, leading to incorrect behavior (e.g., toggleDropdown affecting the wrong element, CSS styles applying incorrectly).

To handle multiple instances, consider:

  1. Generating unique IDs for each instance of the dropdown (e.g., by appending the sequence's unique ID).
  2. Using classes for JavaScript hooks and CSS styling primarily, and then using this.$() to scope selections to the current sequence block's instance of the dropdown.

This is a critical issue if multiple sequence blocks can appear on one page.

this.renderDropdown();
this.bind();
this.render(parseInt(this.el.data('position'), 10));
}

Sequence.prototype.renderDropdown = function() {
// Renders the dropdown when there isn't enough space for all units in the bar
// Hide the dropdown by default and only show if needed.
this.$('#sequence-list > #dropdown-container').hide();
this.$(`#sequence-list > li.sequence-list-item`).show();
// Calculate the number of tabs that can fit comfortably and if the
// number of units is greater we show the dropdown.
const tabListWidth = this.$('#sequence-list').width();
const singleTabWidth = this.$('#sequence-list > li:first').width();
const tabCount = this.$('#sequence-list > li.sequence-list-item').length;
const overFlowCount = Math.floor(tabListWidth / singleTabWidth);
// Reduce 1 to offsets index and another one to accommodate the button
const overFlowIdx = overFlowCount - 2;
const showDropdown = overFlowCount < tabCount;
if (!showDropdown) {
return;
}
// If the dropdown button doesn't exist add it, otherwise move the
// existing button to the correct place.
if (this.$('#sequence-list > #dropdown-container').length === 0) {
// xss-lint: disable=javascript-jquery-insertion
this.$('#sequence-list > li.sequence-list-item').eq(overFlowIdx).after(this.dropdownButtonTpl);
} else {
this.$('#sequence-list > li.sequence-list-item').eq(overFlowIdx)
// xss-lint: disable=javascript-jquery-insertion
.after(this.$('#sequence-list > #dropdown-container'));
}
// Show the dropdown UX and hide all the overflowing unit buttons.
this.$('#sequence-list > #dropdown-container').show();
this.$(`#sequence-list > li.sequence-list-item:lt(${overFlowIdx + 1})`).show();
this.$(`#sequence-list > li.sequence-list-item:gt(${overFlowIdx})`).hide();
const dropdownList = this.$('#dropdown-sequence-list > ol');
// The dropdown buttons are modified copies of the unit nav buttons.
dropdownList.empty();
this.$(`#sequence-list > li.sequence-list-item:gt(${overFlowIdx})`).each((idx, el) => {
const cloneEl = $(el).clone();
const navButton = cloneEl.find("button");
const unitTitle = navButton.data('page-title');
navButton.click(self.goto);
navButton.find(".sequence-tooltip").remove();
navButton.find("span.icon").after(
HtmlUtils.joinHtml(HtmlUtils.HTML('<span class="unit-title">'), unitTitle, HtmlUtils.HTML('</span>')).toString()
);
//xss-lint: disable=javascript-jquery-insert-into-target
cloneEl.show().appendTo(dropdownList);
});
}

Sequence.prototype.$ = function(selector) {
return $(selector, this.el);
};

Sequence.prototype.bind = function() {
this.$('#sequence-list .nav-item').click(this.goto);
$(document).click(this.handleClickOutsideDropdown);
this.$('#dropdown-sequence-list .dropdown-item').click(this.goto);
Comment thread
xitij2000 marked this conversation as resolved.
this.$('#dropdown-sequence-list-button').click(this.toggleDropdown);
this.$('#sequence-list .nav-item').keypress(this.keyDownHandler);
this.el.on('bookmark:add', this.addBookmarkIconToActiveNavItem);
this.el.on('bookmark:remove', this.removeBookmarkIconFromActiveNavItem);
this.$('#sequence-list .nav-item').on('focus mouseenter', this.displayTabTooltip);
this.$('#sequence-list .nav-item').on('blur mouseleave', this.hideTabTooltip);
$(window).on('resize', _.debounce(this.renderDropdown.bind(this), 200));
};

Sequence.prototype.handleClickOutsideDropdown = function(event) {
if(!this.$('#dropdown-container')?.[0]?.contains(event.target)) {
this.$('#dropdown-sequence-list').hide();
}
}

Sequence.prototype.toggleDropdown = function() {
$('#dropdown-sequence-list').toggle();
Comment thread
xitij2000 marked this conversation as resolved.
}

Sequence.prototype.previousNav = function(focused, index) {
var $navItemList,
$sequenceList = $(focused).parent().parent();
Expand Down Expand Up @@ -289,6 +364,7 @@
Sequence.prototype.goto = function(event) {
var alertTemplate, alertText, isBottomNav, newPosition, widgetPlacement;
event.preventDefault();
this.$('#dropdown-sequence-list').hide();

// Links from courseware <a class='seqnav' href='n'>...</a>, was .target_tab
if ($(event.currentTarget).hasClass('seqnav')) {
Expand Down
31 changes: 29 additions & 2 deletions xmodule/static/css-builtin-blocks/SequenceBlockDisplay.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,13 @@
position: relative;
height: 100%;
flex-grow: 1;
max-width: calc(100% - 80px);
}

@media (min-width: 768px) {
.xmodule_display.xmodule_SequenceBlock .sequence-nav .sequence-list-wrapper {
max-width: calc(100% - 160px);
}
}

@media (max-width: 575.98px) {
Expand All @@ -91,7 +98,7 @@
display: flex;
}

.xmodule_display.xmodule_SequenceBlock .sequence-nav ol li {
.xmodule_display.xmodule_SequenceBlock .sequence-nav ol li, .dropdown-toggle {
box-sizing: border-box;
min-width: 40px;
flex-grow: 1;
Expand All @@ -104,7 +111,11 @@
border-right-style: solid;
}

.xmodule_display.xmodule_SequenceBlock .sequence-nav ol li button {
.xmodule_display.xmodule_SequenceBlock .sequence-nav ol li .dropdown-toggle {
height: 49px !important;
Comment on lines +114 to +115

@gemini-code-assist gemini-code-assist Bot Jun 3, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The use of !important here for height: 49px !important; is generally discouraged as it can make CSS harder to maintain and override later.

Is it possible to achieve the desired height by increasing selector specificity or adjusting other conflicting styles, rather than relying on !important?

}

.xmodule_display.xmodule_SequenceBlock .sequence-nav ol li button, .dropdown-toggle {
width: 100%;
height: 49px;
position: relative;
Expand All @@ -119,6 +130,22 @@
overflow: visible;
}

.xmodule_display.xmodule_SequenceBlock .sequence-nav #dropdown-container ol li button {
display: flex;
align-items: center;
padding-left: 0.5rem;
padding-right: 0.5rem;
}

.xmodule_display.xmodule_SequenceBlock .sequence-nav #dropdown-container ol li button .unit-title {
display: flex;
flex-grow: 1;
text-overflow: ellipsis;
overflow: hidden;
white-space: nowrap;
margin: 0 0.5rem;
}

.xmodule_display.xmodule_SequenceBlock .sequence-nav ol li button .icon {
display: inline-block;
line-height: 100%;
Expand Down
Loading