From b01fd7ad81524b5f2773ccba4b5789a6074ffb9d Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Thu, 12 Apr 2018 17:29:54 +0100 Subject: Fixes unresolved discussions rendering the error state instead of the diff --- app/assets/javascripts/notes.js | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) (limited to 'app/assets/javascripts/notes.js') diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index ac70ddb3ff4..184f335809d 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -19,7 +19,7 @@ import AjaxCache from '~/lib/utils/ajax_cache'; import Vue from 'vue'; import syntaxHighlight from '~/syntax_highlight'; import SkeletonLoadingContainer from '~/vue_shared/components/skeleton_loading_container.vue'; -import { __ } from '~/locale'; +import { __, sprintf } from '~/locale'; import axios from './lib/utils/axios_utils'; import { getLocationHash } from './lib/utils/url_utility'; import Flash from './flash'; @@ -1434,10 +1434,11 @@ export default class Notes { static renderDiffError($container) { $container.find('.line_content').html( $(` -
- ${__( - 'Unable to load the diff.', - )} Try again? +
+ ${sprintf(__('Unable to load the diff.%{buttonStartTag}Try again%{buttonEndTag}?'), { + buttonStartTag: '' + }, false)}
`), ); @@ -1455,7 +1456,12 @@ export default class Notes { const fileHolder = $container.find('.file-holder'); const url = fileHolder.data('linesPath'); - axios + /** + * We only fetch resolved discussions. + * Unresolved discussions don't have an endpoint being provided. + */ + if (url) { + axios .get(url) .then(({ data }) => { Notes.renderDiffContent($container, data); @@ -1463,6 +1469,7 @@ export default class Notes { .catch(() => { Notes.renderDiffError($container); }); + } } toggleCommitList(e) { -- cgit v1.2.1 From a236b5e6a22cf762354a9bb3460e6bad80ab5b3f Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Fri, 13 Apr 2018 09:16:06 +0100 Subject: Render only one error message per diff Move html to haml file instead of JS --- app/assets/javascripts/notes.js | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) (limited to 'app/assets/javascripts/notes.js') diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 184f335809d..9f1db70b06e 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -1431,31 +1431,21 @@ export default class Notes { syntaxHighlight(fileHolder); } - static renderDiffError($container) { - $container.find('.line_content').html( - $(` -
- ${sprintf(__('Unable to load the diff.%{buttonStartTag}Try again%{buttonEndTag}?'), { - buttonStartTag: '' - }, false)} -
- `), - ); - } - loadLazyDiff(e) { const $container = $(e.currentTarget).closest('.js-toggle-container'); Notes.renderPlaceholderComponent($container); $container.find('.js-toggle-lazy-diff').removeClass('js-toggle-lazy-diff'); - const tableEl = $container.find('tbody'); - if (tableEl.length === 0) return; + const $tableEl = $container.find('tbody'); + if ($tableEl.length === 0) return; const fileHolder = $container.find('.file-holder'); const url = fileHolder.data('linesPath'); + const $errorContainer = $container.find('.js-error-lazy-load-diff'); + const $successContainer = $container.find('.js-success-lazy-load'); + /** * We only fetch resolved discussions. * Unresolved discussions don't have an endpoint being provided. @@ -1464,10 +1454,15 @@ export default class Notes { axios .get(url) .then(({ data }) => { + // Reset state in case last request returned error + $successContainer.removeClass('hidden'); + $errorContainer.addClass('hidden'); + Notes.renderDiffContent($container, data); }) .catch(() => { - Notes.renderDiffError($container); + $successContainer.addClass('hidden'); + $errorContainer.removeClass('hidden'); }); } } -- cgit v1.2.1 From 5b171cee3ac20ccbb591aa5e2fad852fcc7440b8 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Fri, 13 Apr 2018 09:55:47 +0100 Subject: Remove unused imports --- app/assets/javascripts/notes.js | 1 - 1 file changed, 1 deletion(-) (limited to 'app/assets/javascripts/notes.js') diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 9f1db70b06e..a00cf0e239a 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -19,7 +19,6 @@ import AjaxCache from '~/lib/utils/ajax_cache'; import Vue from 'vue'; import syntaxHighlight from '~/syntax_highlight'; import SkeletonLoadingContainer from '~/vue_shared/components/skeleton_loading_container.vue'; -import { __, sprintf } from '~/locale'; import axios from './lib/utils/axios_utils'; import { getLocationHash } from './lib/utils/url_utility'; import Flash from './flash'; -- cgit v1.2.1 From 8f189df86f6ebd2bd4974ce180ed75e9d97cd81e Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Mon, 16 Apr 2018 12:30:46 +0100 Subject: Use different class for try again button in collapsed diffs --- app/assets/javascripts/notes.js | 3 +++ 1 file changed, 3 insertions(+) (limited to 'app/assets/javascripts/notes.js') diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index a00cf0e239a..a4669b1672a 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -197,6 +197,8 @@ export default class Notes { ); this.$wrapperEl.on('click', '.js-toggle-lazy-diff', this.loadLazyDiff); + this.$wrapperEl.on('click', '.js-toggle-lazy-diff-retry-button', this.loadLazyDiff); + // fetch notes when tab becomes visible this.$wrapperEl.on('visibilitychange', this.visibilityChange); // when issue status changes, we need to refresh data @@ -243,6 +245,7 @@ export default class Notes { this.$wrapperEl.off('click', '.js-comment-resolve-button'); this.$wrapperEl.off('click', '.system-note-commit-list-toggler'); this.$wrapperEl.off('click', '.js-toggle-lazy-diff'); + this.$wrapperEl.off('click', '.js-toggle-lazy-diff-retry-button'); this.$wrapperEl.off('ajax:success', '.js-main-target-form'); this.$wrapperEl.off('ajax:success', '.js-discussion-note-form'); this.$wrapperEl.off('ajax:complete', '.js-main-target-form'); -- cgit v1.2.1 From 4b2ff003920cff431ef4e3e8b0436aa924b34fda Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Mon, 16 Apr 2018 16:36:24 +0100 Subject: Disable try again button while fetching the API --- app/assets/javascripts/notes.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'app/assets/javascripts/notes.js') diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index a4669b1672a..a07af3ee93a 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -197,7 +197,7 @@ export default class Notes { ); this.$wrapperEl.on('click', '.js-toggle-lazy-diff', this.loadLazyDiff); - this.$wrapperEl.on('click', '.js-toggle-lazy-diff-retry-button', this.loadLazyDiff); + this.$wrapperEl.on('click', '.js-toggle-lazy-diff-retry-button', this.onClickRetryLazyLoad.bind(this)); // fetch notes when tab becomes visible this.$wrapperEl.on('visibilitychange', this.visibilityChange); @@ -1433,6 +1433,17 @@ export default class Notes { syntaxHighlight(fileHolder); } + onClickRetryLazyLoad(e) { + const $retryButton = $(e.currentTarget); + + $retryButton.prop('disabled', true); + + return this.loadLazyDiff(e) + .then(() => { + $retryButton.prop('disabled', false); + }); + } + loadLazyDiff(e) { const $container = $(e.currentTarget).closest('.js-toggle-container'); Notes.renderPlaceholderComponent($container); @@ -1453,7 +1464,7 @@ export default class Notes { * Unresolved discussions don't have an endpoint being provided. */ if (url) { - axios + return axios .get(url) .then(({ data }) => { // Reset state in case last request returned error @@ -1467,6 +1478,7 @@ export default class Notes { $errorContainer.removeClass('hidden'); }); } + return Promise.resolve(); } toggleCommitList(e) { -- cgit v1.2.1