Skip to content
This repository has been archived by the owner on Feb 22, 2024. It is now read-only.

Abillity to collapse rubrics when grading #467

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LiamClark
Copy link
Contributor

The current implementation has some performance problems on collapsing the table.

fixes #460

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 55.993% when pulling 5725698 on LiamClark:collapse-rubrics into b9badfc on devhub-tud:master.

Copy link
Member

@Fastjur Fastjur left a comment

Choose a reason for hiding this comment

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

ezgif com-video-to-gif

There are indeed some performance issues. Also the state of the arrow seems to be inverted. And the first time we click it it doesn't seem to respond.

Perhaps the initial state is simply incorrect?

@jwgmeligmeyling
Copy link
Member

Yup the performance issues are weird. Liam is currently profiling that. It could be that the set of elements is much larger than thought, or that the event handler is invoked multiple times. But it seems the code is just fine and that the problem is actually caused by the fact that collapse is generally slow in Webkit. In the latter case, having this feature not performing is still better than not having this feature at all I suppose...

var down = "octicon-chevron-down"
var right ="octicon-chevron-right"

$('[data-target*="collapse-target"]').click(function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this query-selector is rather expensive. It might be better to instead add a class collapse-target and do document.querySelector('.collapse-target') and then put the actual value in data-target="${task?index}". You can then retrieve the index with tableRow.dataset.target

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make position of Remarks text box absolute to screen
5 participants