Skip to content
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.

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

show error-view instead of yellow notification on content loading errors #57

Merged
merged 1 commit into from
Sep 18, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 10 additions & 3 deletions js/controller/foldercontroller.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ define(function(require) {
var _ = require('underscore');
var Radio = require('radio');
var FolderService = require('service/folderservice');
var ErrorMessageFactory = require('util/errormessagefactory');

Radio.message.on('fetch:bodies', fetchBodies);

Expand All @@ -45,6 +46,7 @@ define(function(require) {
* @param {Account} account
* @param {Folder} folder
* @param {boolean} noSelect
* @param {string} searchQuery
* @returns {undefined}
*/
function loadFolderMessages(account, folder, noSelect, searchQuery) {
Expand Down Expand Up @@ -108,10 +110,15 @@ define(function(require) {
});

$.when(loadingMessages).fail(function() {
var icon;
if (folder.get('specialRole')) {
icon = 'icon-' + folder.get('specialRole');
}
Radio.ui.trigger('content:error', ErrorMessageFactory.getRandomFolderErrorMessage(folder), icon);

// Set the old folder as being active
var folder = require('state').currentFolder;
Radio.folder.trigger('setactive', account, folder);
Radio.ui.trigger('error:show', t('mail', 'Error while loading messages.'));
var oldFolder = require('state').currentFolder;
Radio.folder.trigger('setactive', account, oldFolder);
});
}
}
Expand Down
3 changes: 2 additions & 1 deletion js/controller/messagecontroller.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ define(function(require) {
var _ = require('underscore');
var OC = require('OC');
var Radio = require('radio');
var ErrorMessageFactory = require('util/errormessagefactory');

Radio.message.on('load', function(account, folder, message, options) {
//FIXME: don't rely on global state vars
Expand Down Expand Up @@ -101,7 +102,7 @@ define(function(require) {
}
});
$.when(fetchingMessage).fail(function() {
Radio.ui.trigger('error:show', t('mail', 'Error while loading the selected message.'));
Radio.ui.trigger('message:error', ErrorMessageFactory.getRandomMessageErrorMessage());
});
}

Expand Down
4 changes: 4 additions & 0 deletions js/templates/error.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<div class="">
{{#if icon}}<div class="{{icon}}"></div>{{/if}}
<h2>{{{ text }}}</h2>
</div>
79 changes: 79 additions & 0 deletions js/util/errormessagefactory.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/**
* @author Christoph Wurst <christoph@winzerhof-wurst.at>
*
* Mail
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/

define(function(require) {
'use strict';

var smileys = [
':-(',
':-/',
':-\\',
':-|',
':\'-(',
':\'-/',
':\'-\\',
':\'-|'
];

function getRandomSmiley() {
return smileys[Math.floor(Math.random() * smileys.length)]
}

/**
* @param {Folder} folder
* @returns {string}
*/
function getRandomFolderErrorMessage(folder) {
var folderName = folder.get('name');
var rawTexts = [
t('mail', 'Could not load {tag}{name}{endtag}', {
name: folderName
}),
t('mail', 'We couldn’t load {tag}{name}{endtag}', {
name: folderName
}),
t('mail', 'There was a problem loading {tag}{name}{endtag}', {
name: folderName
})
];
var texts = _.map(rawTexts, function(text) {
return text.replace('{tag}', '<strong>').replace('{endtag}', '</strong>');
Copy link
Member

Choose a reason for hiding this comment

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

Are you trying to bold the folder name here?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes

Copy link
Member

Choose a reason for hiding this comment

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

see #57 (comment) :)
If the strong is correctly outputted on the html and still isn't displayed bold, this could be because the strong css value "font-weight" has been reset. If so just use a span and set it in css. Should be enough! ;)

Copy link
Member

Choose a reason for hiding this comment

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

Riight, we need to add in our CSS:
strong { font-weight: 600; }

@skjnldsv great to see you back here by the way! :)

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Glad to see you both! :)

});
var text = texts[Math.floor(Math.random() * texts.length)]
return text + ' ' + getRandomSmiley();
}

/**
* @returns {string}
*/
function getRandomMessageErrorMessage() {
var texts = [
t('mail', 'We couldn’t load your message'),
t('mail', 'Unable to load the desired message'),
t('mail', 'There was a problem loading the message')
];
var text = texts[Math.floor(Math.random() * texts.length)]
return text + ' ' + getRandomSmiley();
}

return {
getRandomFolderErrorMessage: getRandomFolderErrorMessage,
getRandomMessageErrorMessage: getRandomMessageErrorMessage
};
});
10 changes: 10 additions & 0 deletions js/views/appview.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ define(function(require) {
var FolderContentView = require('views/foldercontent');
var NavigationAccountsView = require('views/navigation-accounts');
var SettingsView = require('views/settings');
var ErrorView = require('views/errorview');
var LoadingView = require('views/loadingview');
var NavigationView = require('views/navigation');
var SetupView = require('views/setup');
Expand All @@ -30,6 +31,7 @@ define(function(require) {
require('views/helper');

var ContentType = Object.freeze({
ERROR: -2,
LOADING: -1,
FOLDER_CONTENT: 0,
SETUP: 1,
Expand All @@ -54,6 +56,7 @@ define(function(require) {
this.listenTo(Radio.ui, 'error:show', this.showError);
this.listenTo(Radio.ui, 'setup:show', this.showSetup);
this.listenTo(Radio.ui, 'foldercontent:show', this.showFolderContent);
this.listenTo(Radio.ui, 'content:error', this.showContentError);
this.listenTo(Radio.ui, 'content:loading', this.showContentLoading);
this.listenTo(Radio.ui, 'title:update', this.updateTitle);
this.listenTo(Radio.ui, 'accountsettings:show', this.showAccountSettings);
Expand Down Expand Up @@ -165,6 +168,13 @@ define(function(require) {

this.content.show(new FolderContentView(options));
},
showContentError: function(text, icon) {
this.activeContent = ContentType.ERROR;
this.content.show(new ErrorView({
text: text,
icon: icon
}));
},
showContentLoading: function(text) {
this.activeContent = ContentType.LOADING;
this.content.show(new LoadingView({
Expand Down
44 changes: 44 additions & 0 deletions js/views/errorview.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/**
* @author Christoph Wurst <christoph@winzerhof-wurst.at>
*
* Mail
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/

define(function(require) {
'use strict';

var Handlebars = require('handlebars');
var Marionette = require('marionette');
var ErrorTemplate = require('text!templates/error.html');

var ErrorView = Marionette.ItemView.extend({
id: 'emptycontent',
className: 'container',
template: Handlebars.compile(ErrorTemplate),
templateHelpers: function() {
return {
text: this.text,
icon: this.icon
};
},
initialize: function(options) {
this.text = options.text || t('mail', 'An unknown error occurred');
this.icon = options.icon || 'icon-mail';
}
});

return ErrorView;
});
9 changes: 9 additions & 0 deletions js/views/foldercontent.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@ define(function(require) {
var ComposerView = require('views/composer');
var MessageView = require('views/message');
var MessagesView = require('views/messagesview');
var ErrorView = require('views/errorview');
var LoadingView = require('views/loadingview');
var MessageContentTemplate = require('text!templates/foldercontent.html');

var DetailView = Object.freeze({
ERROR: -2,
MESSAGE: 1,
COMPOSER: 2
});
Expand All @@ -55,6 +57,7 @@ define(function(require) {
this.searchQuery = options.searchQuery;

this.listenTo(Radio.ui, 'message:show', this.onShowMessage);
this.listenTo(Radio.ui, 'message:error', this.onShowError);
this.listenTo(Radio.ui, 'composer:show', this.onShowComposer);
this.listenTo(Radio.ui, 'composer:leave', this.onComposerLeave);
this.listenTo(Radio.keyboard, 'keyup', this.onKeyUp);
Expand Down Expand Up @@ -89,6 +92,12 @@ define(function(require) {

Radio.ui.trigger('messagesview:messageflag:set', message.id, 'unseen', false);
},
onShowError: function(errorMessage) {
this.message.show(new ErrorView({
text: errorMessage
}));
this.detailView = DetailView.ERROR;
},
onShowComposer: function(data) {
$('.tooltip').remove();
$('#mail_new_message').prop('disabled', true);
Expand Down
4 changes: 3 additions & 1 deletion js/views/navigation-accounts.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ define(function(require) {
});
});

folder.set('active', true);
if (folder) {
Copy link
Member Author

Choose a reason for hiding this comment

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

small bugfix: if the first folder fails loading, folder was undefined here and therefore an error was shown in the js console.

folder.set('active', true);
}
},
/**
* @returns {undefined}
Expand Down