') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); add a promise to save and update everything in the difftracker before… by marcellamaki · Pull Request #2879 · learningequality/studio · GitHub
Skip to content

add a promise to save and update everything in the difftracker before… - #2879

Merged
rtibbles merged 3 commits into
learningequality:hotfixesfrom
marcellamaki:0-incomplete-resources-validation
Jan 29, 2021
Merged

add a promise to save and update everything in the difftracker before…#2879
rtibbles merged 3 commits into
learningequality:hotfixesfrom
marcellamaki:0-incomplete-resources-validation

Conversation

@marcellamaki

@marcellamakimarcellamaki commented Jan 27, 2021

Copy link
Copy Markdown
Member

Description

This PR refactors some code in the DetailsTabView to create an immediateSaveAll, that makes sure the content of the diffTracker is all updated before the Edit Modal closes

Issue Addressed (if applicable)

Addresses #2567

Steps to Test

  • In the tree view, create a new exercise.
  • Fill out the title, mastery criteria, and the license field
  • When you fill out the last field, click the "Finish" button within <1s of updating the field. (This can be a little bit tricky. I find it easiest if I do this for the license field so I only have to update the

rtibbles
rtibbles reviewed Jan 27, 2021
View reviewed changes
Comment threadcontentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue Outdated
this.$nextTick(this.handleValidation);
},
},
closingModal: function(newValue) {

@rtibblesrtibblesJan 27, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am not sure that this guarantees that the saves will complete before the teardown occurs - we have a promise returned from the immediateSaveAll, but we're not waiting on it to resolve anywhere.

@marcellamakimarcellamakiJan 27, 2021

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

You're right - the reason this is working okay in the preliminary manual testing I was doing is because it doesn't actually need to resolve for the updates to be made in that case. I'll think on this...

@marcellamaki
marcellamaki marked this pull request as ready for review January 29, 2021 15:37
rtibbles
rtibbles requested changes Jan 29, 2021
View reviewed changes

@rtibblesrtibbles left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Closer, but still a couple of things that I think might not be working as you intend.

Comment threadcontentcuration/contentcuration/frontend/channelEdit/components/edit/EditView.vue Outdated
* before the validation pop up is executed
*/
immediateSaveAll: function() {
this.$refs.detailsTab.immediateSaveAll();

@rtibblesrtibblesJan 29, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should return here, rather than returning a dummy promise on the next line here - otherwise we are still not waiting on the completion of immediateSaveAll

Comment threadcontentcuration/contentcuration/frontend/channelEdit/components/edit/EditView.vue Outdated
:key="nodeIds.join('-')"
ref="detailsTab"
:nodeIds="nodeIds"
:closingModal="closingModal"

@rtibblesrtibblesJan 29, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This doesn't seem to be a prop on DetailsTabView any more, so I think this line is vestigial and can be removed.

Comment threadcontentcuration/contentcuration/frontend/channelEdit/components/edit/EditView.vue Outdated
return !this.oneSelected || this.getContentNodeDetailsAreValid(this.nodeIds[0]);
// only run this if the modal isn't closing
// because async DOM updates during close can cause issues
if (!this.closingModal) {

@rtibblesrtibblesJan 29, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In EditModal, closingModal starts out as false at initialization, and the only line that sets its value in there also sets it to false, so I don't think this guard is having any effect.

If it is working fine at the moment, I'd recommend just removing all the closingModal references and thus simplify the diff.

rtibbles
rtibbles approved these changes Jan 29, 2021
View reviewed changes

@rtibblesrtibbles left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code looks good - and no matter how fast I type and click 'Finish' it doesn't tell me my topic has no title.

@rtibbles
rtibbles merged commit fb16568 into learningequality:hotfixesJan 29, 2021
@rtibblesrtibbles linked an issue Jan 29, 2021 that may be closed by this pull request
"0 incomplete resources found" alert #2567
Closed
@rtibblesrtibbles mentioned this pull request Jan 29, 2021
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rtibblesrtibblesrtibbles approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Post Release Stabilization

Development

Successfully merging this pull request may close these issues.

"0 incomplete resources found" alert

2 participants

@marcellamaki@rtibbles