Fixes styling for civi sepa dashboard - #820
Conversation
… to use the civcirm button template and added icons Project60#816
…also be changed for this table
jensschuppe
left a comment
jensschuppe
left a comment
There was a problem hiding this comment.
Please have a look at the PHPStan and PHP_CodeSniffer issues related to the files affected in this PR.
| @@ -0,0 +1,24 @@ | |||
| .sepa_dashboard str.submit_missed td { | |||
There was a problem hiding this comment.
Typo, should be tr.submit_missed.
| } | ||
|
|
||
| .sepa_dashboard .sepa_actions { | ||
| display: flex; |
There was a problem hiding this comment.
Why does the table cell have to be display: flex;? - this actually causes the buttons to grow in height and shouldn't be necessary.
The buttons are inline-block so text-align: right; should be enough.
There was a problem hiding this comment.
This adresses the table cell containing the buttons. Setting the display property to flex aligns the buttons horizontally instead of vertical. The overall screen space the buttons take up is reduced.
Nonetheless the selector could be more descriptive.
| background-color: var(--crm-layer2-bg-color, #00830033); | ||
| } | ||
|
|
||
| .sepa_dashboard .sepa_actions { |
There was a problem hiding this comment.
Let's use BEM conventions for selectors, making this .sepa-dashboard__sepa-actions.
| <th class="sorting" aria-controls="sepa-option">{ts domain="org.project60.sepa"}Group Name{/ts}</th> | ||
| <th class="sorting" aria-controls="sepa-option">{ts domain="org.project60.sepa"}Status{/ts}</th> | ||
| <th class="sorting" aria-controls="sepa-option">{ts domain="org.project60.sepa"}Type{/ts}</th> | ||
| <th class="sorting" aria-controls="sepa-option">{ts domain="org.project60.sepa"}Submission{/ts}</th> | ||
| <th class="sorting" aria-controls="sepa-option">{ts domain="org.project60.sepa"}Collection{/ts}</th> | ||
| <th class="sorting" aria-controls="sepa-option">{ts domain="org.project60.sepa"}Transactions{/ts}</th> | ||
| <th class="sorting" aria-controls="sepa-option">{ts domain="org.project60.sepa"}Total{/ts}</th> |
There was a problem hiding this comment.
Let's take the chance getting rid of the domain attribute for translations by wrapping the entire template in {crmScope extensionKey='org.project60.sepa'}…{/crmScope} tags (applies to all occurences of {ts} in this template).
| <td> | ||
| <a href="{crmURL p="civicrm/sepa/listgroup" q="group_id=$group_id"}" class="button button_view">{ts domain="org.project60.sepa"}Contributions{/ts}</a> | ||
| <td class="sepa_actions"> | ||
| {crmButton href="{crmURL p="civicrm/sepa/listgroup" q="group_id=$group_id"}" class="button_view" title="test" icon="fa-info"}{ts domain="org.project60.sepa"}Contributions{/ts}{/crmButton} |
There was a problem hiding this comment.
{crmButton} doesn't need the {crmURL} tag, it has its own p attribute for a CiviCRM path.
There was a problem hiding this comment.
Thanks for pointing it out I have updated the buttons accordingly.
| <td> | ||
| <a href="{crmURL p="civicrm/sepa/listgroup" q="group_id=$group_id"}" class="button button_view">{ts domain="org.project60.sepa"}Contributions{/ts}</a> | ||
| <td class="sepa_actions"> | ||
| {crmButton href="{crmURL p="civicrm/sepa/listgroup" q="group_id=$group_id"}" class="button_view" title="test" icon="fa-info"}{ts domain="org.project60.sepa"}Contributions{/ts}{/crmButton} |
There was a problem hiding this comment.
The title attribute seems off.
| {if $can_batch} | ||
| {if $group.submit == 'missed'} | ||
| <a href="{crmURL p="civicrm/sepa/closegroup" q="group_id=$group_id&status=missed"}" class="button button_close"> | ||
| {crmButton href="{crmURL p="civicrm/sepa/closegroup" q="group_id=$group_id&status=missed"}" class="button_close" title="{ts domain="org.project60.sepa"}Close and Submit{/ts}" icon="fa-paper-plane"}{ts domain="org.project60.sepa"}Close and Submit{/ts}{/crmButton} |
There was a problem hiding this comment.
Translations within HTML attributes need the escape="htmlattributes" parameter.
| } | ||
| } | ||
|
|
||
| function sepa_civicrm_coreResourceList(&$list, $region) { |
There was a problem hiding this comment.
Let's add type hints and the conventional docblock to make PHPStan happy.
dontub
added
the
status:needs work
3 participants
Added a stylesheet to overwrite the background colour of the civi sepa dashboard. The colour now use the colour danger, warning, info and success colours by the theme and use the original colours as a fallback if no walbrook theme is installed.
Changed the buttons to use the civicrm template from civicrm to align their styling with the rest of Civi. Added styling for the order of those buttons. Added icons to those buttons.
Those changes fix issue #816.