Add inplace edit for timeline name #12000

deb1990 · 2018-04-19T07:32:57Z

Overview

This PR adds the following features to the New Case Type page.

Added ability to edit timeline names inline.
Removed the Advanced Section, because now names can be edited from the tabs directly.
Removed the "Remove" label from the trash icon, because otherwise it was taking lot of space.

Before

After

Technical Details

ang/crmCaseType/activitySetDetails.html file is removed, as we dont need the advanced section anymore.
In ang/crmCaseType/edit.html Remove label is removed from trash icon.
crmEditableTabTitle directive has been created to make the tab title editable also it supports

cancel edit if ESCAPE is pressed
cancel edit if input looses focus
cancel if Cross button is pressed
save if ENTER is pressed
save if Check button is pressed

reneolivo · 2018-04-19T10:20:40Z

Reviewing this PR

reneolivo · 2018-04-19T10:29:17Z

ang/crmCaseType.css

@@ -32,6 +34,14 @@
    width: 10em;
 }

+.crmCaseType-edit-actset {


Please use .crmCaseType__activitySet-label--edit:

no need to shorten the name.

you could either use BEM .crmCaseType__activitySet-label--edit or follow the convention in the file: .crmCaseType .edit-activitySet-label.

Following existing convention, crmCaseType-edit-activity-set

reneolivo · 2018-04-19T10:30:04Z

ang/crmCaseType.css

+    border: 0 !important;
+    border-bottom: 1px solid #666 !important;
+    box-sizing: border-box;
+    float: left;


why is float: left important?

It is not required. Removed. Apologies.

reneolivo · 2018-04-19T10:30:55Z

ang/crmCaseType/edit.html

+        <a href="#acttab-{{$index}}">
+          <span ng-if="!activitySet.isInEditMode">{{ activitySet.label }}</span>
+          <input ng-if="activitySet.isInEditMode"
+            class="crmCaseType-edit-actset"


please refer to the class name comment.

Please refer above comment.

reneolivo · 2018-04-19T10:34:09Z

ang/crmCaseType/edit.html

        <span class="crm-i fa-trash" title="{{ts('Remove')}}"
          ng-hide="activitySet.name == 'standard_timeline'"
-          ng-click="removeItem(caseType.definition.activitySets, activitySet)">{{ts('Remove')}}</span>
+          ng-click="removeItem(caseType.definition.activitySets, activitySet)"></span>


Why did you remove this text? I know it's not visible anyways, but maybe it was there for accessibility reasons (not 100% sure).

It was visible before, but I removed it to save space. It is also mentioned in the DOC
Removed the "Remove" label from the trash icon, because otherwise it was taking lot of space.

Ah right sorry 🤦‍♂️

For accessibility, I suggest adding back a title "Remove Activity Set"

reneolivo · 2018-04-19T10:48:04Z

I have tested the changes locally, and they work as intended:

reneolivo · 2018-04-19T11:50:23Z

(r-explain) PASS
(r-test) UNREVIEWED
(r-code) PASS
(r-doc) PASS
(r-maint) UNREVIEWED
(r-run) PASS: Tested the solution locally.
(r-user) UNREVIEWED
(r-tech) PASS: There should be no impact.

jamienovick · 2018-04-20T08:42:18Z

@deb1990 please see core example of edit in place - we should use the same approach:

This can be found on a clean CiviCRM on the activity tab of the contact record.

We should also make sure this change we are making is suitable for Shoreditch theme.

colemanw · 2018-04-20T17:48:14Z

Since this is an Angular context, I think we should move this code into core and out of the Case extension, as it is generally applicable.
https://github.com/civicrm/org.civicrm.civicase/blob/master/ang/civicase/Utils.js#L402-L432

reneolivo · 2018-05-02T09:55:17Z

@colemanw @jamienovick I was investigating about the crm-editable component that already exists in CiviCase and that is also available through CRM.$('...').crmEditable(), but that functionality is a bit different than what we have here.

crm-editable expects to edit a field from an existing entity (for example: the display name of a contact) and it will automatically save the field to the corresponding entity.

In this case, the activity set name is not part of an entity per se, but part of some definition data that is stored as XML for the case type.

The solution I'm providing in this commit: adea3e2 makes the activity set name to look a bit more like crm-editable:

Please let me know if this is satisfactory.

jamienovick · 2018-05-02T11:10:53Z

I'm happy if Coleman is. Looks good great job.

colemanw · 2018-05-08T19:27:54Z

I tried it out and the UX is slightly odd. I would expect that:

Clicking outside the box should cancel the edit (it doesn't)
Pressing ESC should cancel the edit (it doesn't)
The text inside the box should be selectable (clicking and dragging on the text does not select it, instead it pastes)

colemanw · 2018-05-14T18:44:38Z

Pressing enter ought to save instead of creating a line break.
The cancel button is missing (see standard in-place edit UI).

reneolivo

When you click twice on the title it exits the edit mode. Can this be fixed?

deb1990 · 2018-05-15T10:47:30Z

When you click twice on the title it exits the edit mode. Can this be fixed?

@reneolivo this is fixed.

reneolivo

All good now

deb1990 · 2018-05-15T10:57:09Z

Thanks @reneolivo
@colemanw Thanks for all the feedback. They have been implemented, please take a look.

colemanw · 2018-05-15T12:36:28Z

ang/crmCaseType.css

+    text-align: center;
+    width: 30px;
+    z-index: 1;
+}


I'm not sure why this is necessary. If you reuse the classes from standard crm-editable then these buttons should "just work" and when a theme like Shoreditch comes in to restyle everything it won't need to make a special extra bit of css just for these 2 buttons.

Resolved this.

colemanw · 2018-05-15T13:34:40Z

(r-explain) Pass
(r-test) Pass
(r-code) Pass
(r-doc) Pass
(r-maint) Pass
(r-run) Pass
(r-user) _Pass
(r-tech) Pass

colemanw · 2018-05-15T16:59:03Z

@reneolivo in the future please do not add merge commits to a PR. If you need to pull in changes from master do git pull origin master --rebase instead of git pull and then git push -f. They mess up PR management.

To tidy things up, I've rebased & squashed this PR into a single commit.

colemanw · 2018-05-15T17:34:39Z

@civicrm-builder retest this please

colemanw · 2018-05-15T20:00:49Z

@civicrm-builder retest this please

eileenmcnaughton · 2018-05-15T23:19:42Z

Test fails look related PhantomJS 2.1.1 (Linux 0.0.0).crmCaseType crmEditableTabTitle when ESCAPE key is pressed while typing.shows the pen icon
PhantomJS 2.1.1 (Linux 0.0.0).crmCaseType crmEditableTabTitle when ESCAPE key is pressed while typing.hides the save and cancel button
PhantomJS 2.1.1 (Linux 0.0.0).crmCaseType crmEditableTabTitle when ESCAPE key is pressed while typing.makes the title non editable
PhantomJS 2.1.1 (Linux 0.0.0).crmCaseType crmEditableTabTitle when ENTER key is pressed while typing.shows the pen icon
PhantomJS 2.1.1 (Linux 0.0.0).crmCaseType crmEditableTabTitle when ENTER key is pressed while typing.hides the save and cancel button
PhantomJS 2.1.1 (Linux 0.0.0).crmCaseType crmEditableTabTitle when ENTER key is pressed while typing.makes the title non editable
PhantomJS 2.1.1 (Linux 0.0.0).crmCaseType crmEditableTabTitle when ENTER key is pressed while typing.updates the title in angular context

deb1990 · 2018-05-16T06:53:30Z

@colemanw @eileenmcnaughton Fixed the unit tests.

eileenmcnaughton · 2018-05-16T09:57:37Z

merging per label

Included in CiviCRM 5.1.2 Core PR: civicrm#12000

Included in CiviCRM 5.3.0 Core PR: civicrm#12000

civicrm-builder added the master label Apr 19, 2018

reneolivo reviewed Apr 19, 2018

View reviewed changes

deb1990 changed the title ~~(WIP) Add inplace edit for timeline name~~ Add inplace edit for timeline name Apr 19, 2018

deb1990 force-pushed the timeline-name-edit-inplace branch from e37530e to 9a99ed2 Compare May 14, 2018 09:25

deb1990 force-pushed the timeline-name-edit-inplace branch from edab16b to ec3fc22 Compare May 15, 2018 06:39

reneolivo approved these changes May 15, 2018

View reviewed changes

reneolivo suggested changes May 15, 2018

View reviewed changes

deb1990 force-pushed the timeline-name-edit-inplace branch from ec3fc22 to 43740a0 Compare May 15, 2018 10:46

reneolivo approved these changes May 15, 2018

View reviewed changes

colemanw reviewed May 15, 2018

View reviewed changes

colemanw added the merge on pass label May 15, 2018

Add inplace edit for timeline name

d7a470d

colemanw force-pushed the timeline-name-edit-inplace branch from ffdd363 to d7a470d Compare May 15, 2018 17:31

Fix unit tests

7c369cd

eileenmcnaughton merged commit bd27f44 into civicrm:master May 16, 2018

deb1990 added a commit to compucorp/civicrm-core that referenced this pull request May 17, 2018

PCHR-3584: Add inplace edit for timeline name

c9766f6

Included in CiviCRM 5.1.2 Core PR: civicrm#12000

This was referenced May 17, 2018

PCHR-3584: Add inplace edit for timeline name compucorp/civicrm-core#20

Merged

PCHR-3584: Timeline name edit in place style fixes compucorp/org.civicrm.shoreditch#109

Merged

deb1990 added a commit to compucorp/civicrm-core that referenced this pull request May 18, 2018

PCHR-3584: Add inplace edit for timeline name

66ca19b

Included in CiviCRM 5.1.2 Core PR: civicrm#12000

deb1990 added a commit to compucorp/civicrm-core that referenced this pull request May 18, 2018

PCHR-3584: Add inplace edit for timeline name

f6550d1

Included in CiviCRM 5.3.0 Core PR: civicrm#12000

deb1990 mentioned this pull request May 18, 2018

PCHR-3584: Add inplace edit for timeline name compucorp/civicrm-core#22

Merged

deb1990 added a commit to compucorp/civicrm-core that referenced this pull request May 18, 2018

PCHR-3584: Add inplace edit for timeline name

d59a8a2

Included in CiviCRM 5.3.0 Core PR: civicrm#12000

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add inplace edit for timeline name #12000

Add inplace edit for timeline name #12000

deb1990 commented Apr 19, 2018 •

edited

Loading

reneolivo commented Apr 19, 2018

reneolivo Apr 19, 2018

deb1990 Apr 19, 2018

reneolivo Apr 19, 2018

deb1990 Apr 19, 2018

reneolivo Apr 19, 2018

deb1990 Apr 19, 2018

reneolivo Apr 19, 2018

deb1990 Apr 19, 2018

reneolivo Apr 19, 2018

colemanw Apr 20, 2018

reneolivo commented Apr 19, 2018

reneolivo commented Apr 19, 2018

jamienovick commented Apr 20, 2018 •

edited

Loading

colemanw commented Apr 20, 2018

reneolivo commented May 2, 2018

jamienovick commented May 2, 2018

colemanw commented May 8, 2018

colemanw commented May 14, 2018

reneolivo left a comment

deb1990 commented May 15, 2018

reneolivo left a comment

deb1990 commented May 15, 2018

colemanw May 15, 2018

colemanw May 15, 2018

deb1990 May 16, 2018

colemanw commented May 15, 2018

colemanw commented May 15, 2018 •

edited

Loading

colemanw commented May 15, 2018

colemanw commented May 15, 2018

eileenmcnaughton commented May 15, 2018

deb1990 commented May 16, 2018

eileenmcnaughton commented May 16, 2018

Add inplace edit for timeline name #12000

Add inplace edit for timeline name #12000

Conversation

deb1990 commented Apr 19, 2018 • edited Loading

Overview

Before

After

Technical Details

reneolivo commented Apr 19, 2018

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

reneolivo commented Apr 19, 2018

reneolivo commented Apr 19, 2018

jamienovick commented Apr 20, 2018 • edited Loading

colemanw commented Apr 20, 2018

reneolivo commented May 2, 2018

jamienovick commented May 2, 2018

colemanw commented May 8, 2018

colemanw commented May 14, 2018

reneolivo left a comment

Choose a reason for hiding this comment

deb1990 commented May 15, 2018

reneolivo left a comment

Choose a reason for hiding this comment

deb1990 commented May 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colemanw commented May 15, 2018

colemanw commented May 15, 2018 • edited Loading

colemanw commented May 15, 2018

colemanw commented May 15, 2018

eileenmcnaughton commented May 15, 2018

deb1990 commented May 16, 2018

eileenmcnaughton commented May 16, 2018

deb1990 commented Apr 19, 2018 •

edited

Loading

jamienovick commented Apr 20, 2018 •

edited

Loading

colemanw commented May 15, 2018 •

edited

Loading