Corrections to creation dialog #154
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,20 +2,11 @@ | |
| import {RocketChat} from 'meteor/rocketchat:lib'; | ||
| import {FlowRouter} from 'meteor/kadira:flow-router'; | ||
| import {ReactiveVar} from 'meteor/reactive-var'; | ||
|
There was a problem hiding this comment. Moved this to the place where the other validation errors are shown |
||
|
There was a problem hiding this comment. moved this to the template instance ('onCreated'), see http://blazejs.org/guide/reusable-components.html#Attach-functions-to-the-instance |
||
|
|
||
| const acEvents = { | ||
| 'click .rc-popup-list__item'(e, t) { | ||
| t.ac.onItemClick(this, e); | ||
| t.debounceValidateExpertise(this.item.name); | ||
|
There was a problem hiding this comment. validation only needs to happen once an item is selected... |
||
| }, | ||
| 'keydown [name="expertise"]'(e, t) { | ||
| t.ac.onKeyDown(e); | ||
|
|
@@ -28,6 +19,7 @@ const acEvents = { | |
| }, | ||
| 'blur [name="expertise"]'(e, t) { | ||
| t.ac.onBlur(e); | ||
| t.debounceValidateExpertise(e.target.value); | ||
|
There was a problem hiding this comment. ... or if the autocomplete vanishes (tab, click outside) |
||
| } | ||
| }; | ||
|
|
||
|
|
@@ -41,9 +33,6 @@ Template.AssistifyCreateRequest.helpers({ | |
| items() { | ||
| return Template.instance().ac.filteredList(); | ||
| }, | ||
| config() { | ||
| const filter = Template.instance().expertise; | ||
| return { | ||
|
|
@@ -61,44 +50,24 @@ Template.AssistifyCreateRequest.helpers({ | |
| createIsDisabled() { | ||
| const instance = Template.instance(); | ||
|
|
||
| if (instance.validExpertise.get() && !instance.titleError.get()) { | ||
| return ''; | ||
| } else { | ||
| return 'disabled'; | ||
| } | ||
| }, | ||
| expertiseError() { | ||
| const instance = Template.instance(); | ||
| return instance.expertiseError.get(); | ||
| }, | ||
| expertise() { | ||
| const instance = Template.instance(); | ||
| return instance.expertise.get(); | ||
| }, | ||
| titleError() { | ||
| const instance = Template.instance(); | ||
| return instance.titleError.get(); | ||
| } | ||
| }); | ||
|
|
||
|
|
||
|
|
@@ -109,53 +78,26 @@ Template.AssistifyCreateRequest.events({ | |
| const position = input.selectionEnd || input.selectionStart; | ||
| const length = input.value.length; | ||
| document.activeElement === input && e && /input/i.test(e.type) && (input.selectionEnd = position + input.value.length - length); | ||
| t.expertise.set(input.value); | ||
|
There was a problem hiding this comment. The copying of the actual value should happen immediately in order to have the value at hand for subsequent calls |
||
| t.validExpertise.set(false); | ||
|
There was a problem hiding this comment. Similarly, the invalidation has to happen immediately in order to prevent inconsistent procedure |
||
| t.expertiseError.set(''); | ||
| }, | ||
| 'input #request_title'(e, t) { | ||
| const input = e.target; | ||
| if (!input.value) { | ||
| t.titleError.set(''); | ||
| } else { | ||
| t.requestTitle.set(input.value); | ||
| t.debounceValidateRequestName(input.value); | ||
|
There was a problem hiding this comment. All the validation should happen inside the validation method - even if it's just about allowing initial values |
||
| } | ||
|
|
||
| }, | ||
| 'input #first_question'(e, t) { | ||
| const input = e.target; | ||
| if (input.value) { | ||
| t.openingQuestion.set(input.value); | ||
| } else { | ||
| t.openingQuestion.set(''); | ||
| } | ||
| }, | ||
| 'submit create-channel__content, click .js-save-request'(event, instance) { | ||
|
|
@@ -164,36 +106,34 @@ Template.AssistifyCreateRequest.events({ | |
| const requestTitle = instance.requestTitle.get(); | ||
| const openingQuestion = instance.openingQuestion.get(); | ||
| if (expertise) { | ||
| instance.titleError.set(null); | ||
| Meteor.call('createRequest', requestTitle, expertise, openingQuestion, (err, result) => { | ||
| if (err) { | ||
| console.log(err); | ||
| switch (err.error) { | ||
| case 'error-invalid-name': | ||
| instance.titleError.set(TAPi18n.__('Invalid_room_name', `${ expertise }...`)); | ||
| return; | ||
| case 'error-duplicate-channel-name': | ||
| instance.titleError.set(TAPi18n.__('Request_already_exists')); | ||
| return; | ||
| case 'error-archived-duplicate-name': | ||
| instance.titleError.set(TAPi18n.__('Duplicate_archived_channel_name', name)); | ||
| return; | ||
| case 'error-invalid-room-name': | ||
| console.log('room name slug error'); | ||
| // toastr.error(TAPi18n.__('Duplicate_archived_channel_name', name)); | ||
| instance.titleError.set(TAPi18n.__('Invalid_room_name', err.details.channel_name)); | ||
| return; | ||
| default: | ||
| return handleError(err); | ||
| } | ||
| } | ||
| console.log('Room Created'); | ||
| // toastr.success(TAPi18n.__('New_request_created')); | ||
| const roomCreated = RocketChat.models.Rooms.findOne({_id: result.rid}); | ||
| FlowRouter.go('request', {name: roomCreated.name}, FlowRouter.current().queryParams); | ||
| }); | ||
| } | ||
| } | ||
| }); | ||
|
|
@@ -206,7 +146,7 @@ Template.AssistifyCreateRequest.onRendered(function() { | |
| this.ac.$element.on('autocompleteselect', function(e, {item}) { | ||
| instance.expertise.set(item.name); | ||
| $('input[name="expertise"]').val(item.name); | ||
| instance.debounceValidateExpertise(item.name); | ||
|
|
||
| return instance.find('.js-save-request').focus(); | ||
| }); | ||
|
|
@@ -217,46 +157,60 @@ Template.AssistifyCreateRequest.onCreated(function() { | |
|
|
||
| instance.expertise = new ReactiveVar(''); //the value of the text field | ||
| instance.validExpertise = new ReactiveVar(false); | ||
| instance.expertiseError = new ReactiveVar(null); | ||
| instance.titleError = new ReactiveVar(null); | ||
| instance.requestTitle = new ReactiveVar(''); | ||
| instance.openingQuestion = new ReactiveVar(''); | ||
|
There was a problem hiding this comment. Validation methods should have the focus on the object which is validated, not the steps of the validation themselves |
||
|
|
||
| instance.debounceValidateExpertise = _.debounce((expertise) => { | ||
| if (!expertise) { | ||
| return false; //expertise is mandatory | ||
| } | ||
| return Meteor.call('assistify:isValidExpertise', expertise, (error, result) => { | ||
| if (error) { | ||
| instance.validExpertise.set(false); | ||
| } else { | ||
| instance.validExpertise.set(result); | ||
| if (!result) { | ||
| instance.expertiseError.set('Expertise_does_not_exist'); | ||
| } else { | ||
| instance.expertiseError.set(''); | ||
| } | ||
| } | ||
| }); | ||
| }, 500); | ||
|
|
||
| instance.debounceValidateRequestName = _.debounce((name) => { | ||
| instance.titleError.set(''); | ||
| if (!name) { | ||
| return; //"none" is a valid name | ||
| } | ||
| if (RocketChat.settings.get('UI_Allow_room_names_with_special_chars')) { | ||
| Meteor.call('roomDisplayNameExists', name, (error, result) => { | ||
| if (error) { | ||
| return; | ||
| } | ||
| if (result) { | ||
| instance.titleError.set('Request_already_exists'); | ||
| } | ||
| }); | ||
| } else { | ||
| const reg = new RegExp(`^${ RocketChat.settings.get('UTF8_Names_Validation') }$`); | ||
| const passesRegex = name.length === 0 || reg.test(name); | ||
| if (!passesRegex) { | ||
| instance.titleError.set('Request_no_special_char'); | ||
| } else { | ||
| Meteor.call('roomNameExists', name, (error, result) => { | ||
| if (error) { | ||
| return; | ||
| } | ||
| if (result) { | ||
| instance.titleError.set('Request_already_exists'); | ||
| } | ||
| }); | ||
| } | ||
| } | ||
| }, 500); | ||
|
|
||
| // instance.clearForm = function() { | ||
| // instance.requestRoomName.set(''); | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -77,7 +77,7 @@ Meteor.methods({ | |
| if (openingQuestion) { | ||
| const msg = openingQuestion; | ||
| const msgObject = { _id: Random.id(), rid:roomCreateResult.rid, msg}; | ||
| RocketChat.sendMessage(Meteor.user(), msgObject, room); | ||
|
There was a problem hiding this comment. @vickyokrm I missed that one earlier: You didn't pass a proper user-object, but only its name. Thus, the message was not complete which made the "unread"-bar appear and made the Smarti-adapter fail |
||
| } | ||
|
|
||
| const helpRequestId = RocketChat.models.HelpRequests.createForSupportArea(expertise, roomCreateResult.rid, '', environment); | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
having a container for the actual error reduces 'if's
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mrsimpson implemented this at first, but having this approach will propagate one message at a time-point. This will prevent highlighting multiple error messages for a input field. In this case, its sufficient to have this approach work as expected.