core#1346: Tagsets should display vertically in activity/new case forms

[monishdeb]

Overview

Currently when there are more than one tagsets configured for activity and/or case, on the new activity or case form the tagset are listed horizontally. Please check the before/after screenshots for the style fixes:

Before

New Activity form:

New Case form:

After

New Activity form:

New Case form:

Comments

ping @lcdservices @colemanw @eileenmcnaughton

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

@demeritcowboy can you review this?

[demeritcowboy]

Yep I can review.

[demeritcowboy]

Thanks this looks like a good improvement for tags but since some unrelated fields are being moved around the screen I have a question below.

• General standards
  • [r-explain] Issue
    • There doesn't seem to be either a functional or technical reason that the case type/status/start date fields are being moved. Is there a reason? Line 93 in templates/CRM/Case/Form/Case.tpl.
      • And if you have case custom fields this would move case type below the custom fields. If the custom fields are specific to the case type this is upside down to the usual, where the controlling field is usually before the fields that depend on the selection.
      • And also the duration field now seems out of place, being optional and higher up than the required fields.
  • [r-user] PASS
  • [r-doc] PASS
  • [r-run] PASS
• Developer standards
  • [r-tech] PASS
  • [r-code] PASS
  • [r-maint] N/A
  • [r-test] PASS

[yashodha]

@monishdeb @demeritcowboy I think we should move duration under Case Start Date since that is consistent with how we show on Activity screen as well. Again for tagsets for Case they are displayed at the very bottom and for Activity they are above Attachment, Repeat and Follow tabs. Whatever we decide they should be consistent across screens
Case

Activity

Thoughts?

[demeritcowboy]

Could moving fields be a separate lab ticket since there's probably 20 different opinions on how this screen should look? This seemed like it was just about making multiple tagsets stack vertically instead of horizontally on activities, and in fact they were already vertical on open case just left-aligned with the left border. So it doesn't seem like any other fields need to be moved to achieve stacking.

And I'm noticing this has changed the intent of the tableLayout parameter and now it's no longer possible to call the common tagset template to stack horizontally. Maybe that's ok just now tableLayout = true means stack vertically and align closer to the center, and tableLayout = false means stack vertically and align to the left. The only other place in core that tableLayout = true seems to be attachment tags, so they also now stack vertically too, but that seems fine.

[monishdeb]

@demeritcowboy functionality to use tableLayout = true is still intact. Its just mean we are using tabular format to show tagset, so I have used condition to append the table tag only if tableLayout = true. Earlier the horizontal arrangement of tagset seems broken because we ae using div elements inside table. As per the patch we have clearly differentiated the outlook based on tableLayout parameter.

As per UI design, I tried it to be consistent with the other forms like New Activity form here.
But I agree with you that such UI changes should be discussed in a separate ticket.

@yashodha @demeritcowboy will it be ok if I revert the Case tpl changes so not to tweak with the design pattern? Will it be ok to merge then?

[demeritcowboy]

Yes thanks if the line 93 block is put back I think it's good to go.

FYI you might be interested in this which I will put in the extension directory later today: https://gitlab.com/physician-health-program-of-bc/opencaserearranger

[yashodha]

@monishdeb sounds good :)

[monishdeb]

@demeritcowboy @yashodha updated the PR

[yashodha]

@monishdeb thanks, will check

[demeritcowboy]

Hi @yashodha was there something specific you were looking to check since otherwise this one seems good to go?

[yashodha]

@demeritcowboy looks good, merging this.