Corrections to creation dialog #154

mrsimpson · 2017-11-21T19:08:37Z

Propagate first message properly (fixes Since the creation dialog for requests supports entering a question, the first message is not send to smarti anymore #153)
Change "Name" to "Title"
get rid of flashing error message on auto-complete-confirmation with tab

- Propagate first message properly (fixes #153) - Change "Name" to "Title" - get rid of flashing error message on auto-complete-confirmation with tab

mrsimpson · 2017-11-21T19:09:19Z

packages/assistify-help-request/client/views/creationDialog/AssistifyCreateRequest.js

@@ -257,6 +257,9 @@ Template.AssistifyCreateRequest.onCreated(function() {
 	instance.delaySetExpertise = _.debounce((expertise) => {
 		instance.expertise.set(expertise);
 	}, 200);
+	instance.delaySetError = _.debounce((error) => {


Actually, this would be the proper solution: Have all errors in a reactive var which is updated lazily. Not implemented now due to time constraints

mrsimpson · 2017-11-21T19:10:50Z

packages/assistify-help-request/server/methods/createRequest.js

@@ -77,7 +77,7 @@ Meteor.methods({
 		if (openingQuestion) {
 			const msg = openingQuestion;
 			const msgObject = { _id: Random.id(), rid:roomCreateResult.rid, msg};
-			RocketChat.sendMessage([Meteor.user().username], msgObject, room);
+			RocketChat.sendMessage(Meteor.user(), msgObject, room);


@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

- more robust error handling - prevent flashing of validation errors

mrsimpson

@vickyokrm I have refactored a lot of the validation code.
Motivation was uncontrolled flashing of error messages and while doing that, I found some part could be improved with respect to maintainability.
Kindly check the comments and perform testing.
If everything's fine: squash and merge, please.

mrsimpson · 2017-11-22T07:23:26Z

packages/assistify-help-request/client/views/creationDialog/AssistifyCreateRequest.html

-				{{/if}}
-				{{#if invalidTitle}}
-					{{> AssistifyCreateInputError text="Request_no_special_char"}}
+				{{#if titleError}}


having a container for the actual error reduces 'if's

@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.

mrsimpson · 2017-11-22T07:24:02Z

packages/assistify-help-request/client/views/creationDialog/AssistifyCreateRequest.js

@@ -2,20 +2,11 @@
 import {RocketChat} from 'meteor/rocketchat:lib';
 import {FlowRouter} from 'meteor/kadira:flow-router';
 import {ReactiveVar} from 'meteor/reactive-var';
-import toastr from 'toastr';


Moved this to the place where the other validation errors are shown

mrsimpson · 2017-11-22T07:27:01Z

packages/assistify-help-request/client/views/creationDialog/AssistifyCreateRequest.js

@@ -2,20 +2,11 @@
 import {RocketChat} from 'meteor/rocketchat:lib';
 import {FlowRouter} from 'meteor/kadira:flow-router';
 import {ReactiveVar} from 'meteor/reactive-var';
-import toastr from 'toastr';
-
-const validateRequestName = (name) => {


moved this to the template instance ('onCreated'), see http://blazejs.org/guide/reusable-components.html#Attach-functions-to-the-instance

mrsimpson · 2017-11-22T07:27:32Z

packages/assistify-help-request/client/views/creationDialog/AssistifyCreateRequest.js


 const acEvents = {
 	'click .rc-popup-list__item'(e, t) {
 		t.ac.onItemClick(this, e);
+		t.debounceValidateExpertise(this.item.name);


validation only needs to happen once an item is selected...

mrsimpson · 2017-11-22T07:28:02Z

packages/assistify-help-request/client/views/creationDialog/AssistifyCreateRequest.js

@@ -28,6 +19,7 @@ const acEvents = {
 	},
 	'blur [name="expertise"]'(e, t) {
 		t.ac.onBlur(e);
+		t.debounceValidateExpertise(e.target.value);


... or if the autocomplete vanishes (tab, click outside)

mrsimpson · 2017-11-22T07:29:34Z

packages/assistify-help-request/client/views/creationDialog/AssistifyCreateRequest.js

-			t.requestTitleInUse.set(undefined);
-			t.checkRequestName(requestName);
-		}
+		t.expertise.set(input.value);


The copying of the actual value should happen immediately in order to have the value at hand for subsequent calls

mrsimpson · 2017-11-22T07:30:27Z

packages/assistify-help-request/client/views/creationDialog/AssistifyCreateRequest.js

-			t.checkRequestName(requestName);
-		}
+		t.expertise.set(input.value);
+		t.validExpertise.set(false);


Similarly, the invalidation has to happen immediately in order to prevent inconsistent procedure

mrsimpson · 2017-11-22T07:31:37Z

packages/assistify-help-request/client/views/creationDialog/AssistifyCreateRequest.js

-			t.invalidTitle.set(false);
-			t.requestTitleInUse.set(undefined);
+			t.requestTitle.set(input.value);
+			t.debounceValidateRequestName(input.value);


All the validation should happen inside the validation method - even if it's just about allowing initial values

mrsimpson · 2017-11-22T07:33:18Z

packages/assistify-help-request/client/views/creationDialog/AssistifyCreateRequest.js

-	instance.requestTitleInUse = new ReactiveVar(undefined);
-	instance.invalidTitle = new ReactiveVar(false);
-	instance.error = new ReactiveVar(null);
-	instance.checkRequestName = _.debounce((name) => {


Validation methods should have the focus on the object which is validated, not the steps of the validation themselves

vickyokrm

The changes with respect refactoring has been tested
Added a minor UI improvement on the 'Expertise' field to highlight if error.

* Fixed Issues 87, 88 * Feature/#23 title first message to new request (#149) * Request title and first message while creating new request * Make titles of inputs and placeholders more consistent * Fix display issues: - Dropdown overlapped by input field - In English, the width of the creation dialog was not 100%, thus input fields within too narrow * Minor corrections with respect to error handling: - Refactor error display to an own template - Show error if selected expertise on request creation is invalid (not chosen from the dropdown) - Fix positioning of "at" on members selection if invalid * Fixes #151 - Misspelled label "jetzt chaten" (#152) * Corrections to creation dialog (#154) * Corrections to creation dialog - Propagate first message properly (fixes #153) - Change "Name" to "Title" - get rid of flashing error message on auto-complete-confirmation with tab * Corrections to creation dialog - more robust error handling - prevent flashing of validation errors * Fix improper clearing of request title * Invalid Expertise field highlight * #156 Prefill request creation form (#157) * fixes #156 - Pass topic as URL param (URL encoded) as `topic` or `expertise` (same effect) - Title is focused if expertise is passed * Allow pre-filling of title and question as well. This way, fixes #20 : The consumer (e. g. the wiki page) can create a simple form an pass the content as parameter * - Use the copied current() instead of the internal value ``` FlowRouter.current ƒ () { // 248 // We can't trust outside, that's why we clone this … ``` - refactor query-selectors => reuse * use FlowRouter API properly * Setting based permissions - downport (#158) * Allow maintenance of per-setting permissions (cherry picked from commit eed869a) * Implicitly assign and revoke setting group permissions (cherry picked from commit 28b769b) * Improve Display of setting permissions (cherry picked from commit 8523456) * Add path to permission title (cherry picked from commit c87a30d) * Permission to access setting permissions (cherry picked from commit 48b1076) * Adapt wording (cherry picked from commit daccad8) * UI-adaptation: Allow users with permission 'manage-selected-permissions' to see and change the affected settings. However, this is not reactive: Once the permissions for a particular setting are changed, the user needs to log off and on again before it becomes effective in the UI. This is most probably a consequence of the CachedCollection. This collection needed to be changed on permission-change. In the backend however, the permissions become effective immediately. (cherry picked from commit 00e4bb5) * Don't adapt sorting on the client side (cherry picked from commit 9b71b62) * Fix: Apply changed setting permissions reactively (cherry picked from commit 293ad73) * Move setting-based permissions to own collection (cherry picked from commit 8f59f1c) * Unify collections for setting and other permissions again into one (cherry picked from commit 8d923c2) * Get rid of frontend exceptions on changing selected settings (cherry picked from commit a7fdc87) * - Sort permissions by group - Do not try to create permissions for hidden settings in higher-level-callbacks - Remove `setting-permissions` collection - fully integrated into `permissions` (cherry picked from commit f007231) * Harmonize wording in German (cherry picked from commit 5cf5df2) * German language informalized (#160) * German language informalized - Liebe Deutsche, wir kennen euch nun besser. Wir wollen ab jetzt “Du” zu einander sagen 😉 * Update de.i18n.json * Update de.i18n.json * Update de.i18n.json * Allow administration even if user has got only `edit-privileged-setting` but not `view-privileged-setting` * Revert "Fixed Issues 87, 88 (livechat on mobile devices)" (#164) * Create configuration expert role on startup (#159) * Allow maintenance of per-setting permissions (cherry picked from commit eed869a) * Implicitly assign and revoke setting group permissions (cherry picked from commit 28b769b) * Improve Display of setting permissions (cherry picked from commit 8523456) * Add path to permission title (cherry picked from commit c87a30d) * Permission to access setting permissions (cherry picked from commit 48b1076) * Adapt wording (cherry picked from commit daccad8) * UI-adaptation: Allow users with permission 'manage-selected-permissions' to see and change the affected settings. However, this is not reactive: Once the permissions for a particular setting are changed, the user needs to log off and on again before it becomes effective in the UI. This is most probably a consequence of the CachedCollection. This collection needed to be changed on permission-change. In the backend however, the permissions become effective immediately. (cherry picked from commit 00e4bb5) * Don't adapt sorting on the client side (cherry picked from commit 9b71b62) * Fix: Apply changed setting permissions reactively (cherry picked from commit 293ad73) * Move setting-based permissions to own collection (cherry picked from commit 8f59f1c) * Unify collections for setting and other permissions again into one (cherry picked from commit 8d923c2) * Get rid of frontend exceptions on changing selected settings (cherry picked from commit a7fdc87) * - Sort permissions by group - Do not try to create permissions for hidden settings in higher-level-callbacks - Remove `setting-permissions` collection - fully integrated into `permissions` (cherry picked from commit f007231) * Harmonize wording in German (cherry picked from commit 5cf5df2) * add configuration package * Add default role configuration on startup * set default system language to DE * Reduce capabilities of config expert and introduce minor admin * Parted the roles for configuration and managing the rest - Manager - well - manages the application, like a minor admin. Target is that this role is capable of doing everything which is necessary while *regularly* running the application - Config-expert is allowed to customize the application (affecting all users' experience) * Informal german language for our custom texts (#165) * German language informalized - Liebe Deutsche, wir kennen euch nun besser. Wir wollen ab jetzt “Du” zu einander sagen 😉 * German texts of custom enhancement informalized * Bump version to 0.5.0 * Update HISTORY.md

Corrections to creation dialog

98ecf6c

- Propagate first message properly (fixes #153) - Change "Name" to "Title" - get rid of flashing error message on auto-complete-confirmation with tab

mrsimpson self-assigned this Nov 21, 2017

mrsimpson requested a review from vickyokrm November 21, 2017 19:08

ghost added the progress:working label Nov 21, 2017

mrsimpson commented Nov 21, 2017

View reviewed changes

Corrections to creation dialog

44d62d0

- more robust error handling - prevent flashing of validation errors

mrsimpson commented Nov 22, 2017

View reviewed changes

mrsimpson and others added 2 commits November 22, 2017 08:58

Fix improper clearing of request title

464f8bd

Invalid Expertise field highlight

6f83482

ghost assigned vickyokrm Nov 22, 2017

vickyokrm approved these changes Nov 22, 2017

View reviewed changes

vickyokrm merged commit 29dd0a6 into develop Nov 22, 2017

ghost removed the progress:working label Nov 22, 2017

mrsimpson deleted the feature/#153-creation-dialog-corrections branch December 7, 2020 09:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Corrections to creation dialog #154

Corrections to creation dialog #154

mrsimpson commented Nov 21, 2017

mrsimpson Nov 21, 2017

mrsimpson Nov 21, 2017

mrsimpson left a comment

mrsimpson Nov 22, 2017

vickyokrm Nov 22, 2017

mrsimpson Nov 22, 2017

mrsimpson Nov 22, 2017

mrsimpson Nov 22, 2017

mrsimpson Nov 22, 2017

mrsimpson Nov 22, 2017

mrsimpson Nov 22, 2017

mrsimpson Nov 22, 2017

mrsimpson Nov 22, 2017

vickyokrm left a comment

Corrections to creation dialog #154

Corrections to creation dialog #154

Conversation

mrsimpson commented Nov 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrsimpson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vickyokrm left a comment

Choose a reason for hiding this comment