UI improvement (#1829)

* open "create new project/activity" form in modal
* re-use action translation for create action title
* do not pre-select first customer in list - fixes #1828
* sort user preferences in sections for better visual grouping
* show comment date only in hover of user avatar

src/Controller/ProfileController.php

@@ -261,10 +261,20 @@ class ProfileController extends AbstractController
             ]);
         }
 
+        $sections = [];
+
+        /** @var UserPreference $pref */
+        foreach ($profile->getPreferences() as $pref) {
+            if ($pref->isEnabled()) {
+                $sections[$pref->getSection()] = $pref->getSection();
+            }
+        }
+
         return $this->render('user/form.html.twig', [
             'tab' => 'preferences',
             'user' => $profile,
             'form' => $form->createView(),
+            'sections' => $sections
         ]);
     }
 

src/Entity/UserPreference.php

@@ -83,11 +83,12 @@ class UserPreference
      * @var int
      */
     private $order = 1000;
-
     /**
-     * @return int
+     * @var string
      */
-    public function getId()
+    private $section = 'default';
+
+    public function getId(): ?int
     {
         return $this->id;
     }
@@ -264,4 +265,16 @@ class UserPreference
 
         return $this;
     }
+
+    public function setSection(string $section): UserPreference
+    {
+        $this->section = $section;
+
+        return $this;
+    }
+
+    public function getSection(): string
+    {
+        return $this->section;
+    }
 }

src/EventSubscriber/UserPreferenceSubscriber.php

@@ -106,6 +106,7 @@ class UserPreferenceSubscriber implements EventSubscriberInterface
                 ->setName(UserPreference::HOURLY_RATE)
                 ->setValue(0)
                 ->setOrder(100)
+                ->setSection('rate')
                 ->setType(MoneyType::class)
                 ->setEnabled($enableHourlyRate)
                 ->setOptions($hourlyRateOptions)
@@ -115,6 +116,7 @@ class UserPreferenceSubscriber implements EventSubscriberInterface
                 ->setName(UserPreference::INTERNAL_RATE)
                 ->setValue(null)
                 ->setOrder(101)
+                ->setSection('rate')
                 ->setType(MoneyType::class)
                 ->setEnabled($enableHourlyRate)
                 ->setOptions(array_merge($hourlyRateOptions, ['label' => 'label.rate_internal', 'required' => false]))
@@ -124,54 +126,63 @@ class UserPreferenceSubscriber implements EventSubscriberInterface
                 ->setName(UserPreference::TIMEZONE)
                 ->setValue($this->getDefaultTimezone())
                 ->setOrder(200)
+                ->setSection('locale')
                 ->setType(TimezoneType::class),
 
             (new UserPreference())
                 ->setName(UserPreference::LOCALE)
                 ->setValue($this->getDefaultLanguage())
                 ->setOrder(300)
+                ->setSection('locale')
                 ->setType(LanguageType::class),
 
             (new UserPreference())
                 ->setName(UserPreference::SKIN)
                 ->setValue($this->getDefaultTheme())
                 ->setOrder(400)
+                ->setSection('theme')
                 ->setType(SkinType::class),
 
             (new UserPreference())
                 ->setName('theme.layout')
                 ->setValue('fixed')
                 ->setOrder(450)
+                ->setSection('theme')
                 ->setType(ThemeLayoutType::class),
 
             (new UserPreference())
                 ->setName('theme.collapsed_sidebar')
                 ->setValue(false)
                 ->setOrder(500)
+                ->setSection('theme')
                 ->setType(CheckboxType::class),
 
             (new UserPreference())
                 ->setName('calendar.initial_view')
                 ->setValue(CalendarViewType::DEFAULT_VIEW)
                 ->setOrder(600)
+                ->setSection('behaviour')
                 ->setType(CalendarViewType::class),
 
             (new UserPreference())
                 ->setName('login.initial_view')
                 ->setValue(InitialViewType::DEFAULT_VIEW)
                 ->setOrder(700)
+                ->setSection('behaviour')
                 ->setType(InitialViewType::class),
 
             (new UserPreference())
                 ->setName('timesheet.daily_stats')
                 ->setValue(false)
                 ->setOrder(800)
+                ->setSection('behaviour')
                 ->setType(CheckboxType::class),
 
             (new UserPreference())
                 ->setName('timesheet.export_decimal')
                 ->setValue(false)
                 ->setOrder(900)
+                ->setSection('behaviour')
                 ->setType(CheckboxType::class),
         ];
     }
@@ -195,6 +206,7 @@ class UserPreferenceSubscriber implements EventSubscriberInterface
                     ->setEnabled($preference->isEnabled())
                     ->setOptions($preference->getOptions())
                     ->setOrder($preference->getOrder())
+                    ->setSection($preference->getSection())
                 ;
             } else {
                 $user->addPreference($preference);

src/Form/ProjectEditForm.php

@@ -78,6 +78,7 @@ class ProjectEditForm extends AbstractType
                 'required' => false,
             ]))
             ->add('customer', CustomerType::class, [
+                'placeholder' => (null === $id && null === $customer) ? '' : false,
                 'query_builder' => function (CustomerRepository $repo) use ($builder, $customer) {
                     $query = new CustomerFormTypeQuery($customer);
                     $query->setUser($builder->getOption('user'));

templates/activity/details.html.twig

@@ -32,11 +32,11 @@
                     {{ activity.comment|comment2html(true) }}
                 </div>
             {% endif %}
-            <table class="table table-hover">
+            <table class="table table-hover dataTable">
                 {% if not activity.visible %}
                     <tr>
                         <th>{{ 'label.visible'|trans }}</th>
-                        <td>
+                        <td colspan="3">
                             {{ widgets.label_boolean(activity.visible) }}
                         </td>
                     </tr>
@@ -46,10 +46,11 @@
                         <th>{{ 'label.customer'|trans }}</th>
                         <td>
                             {{ widgets.label_customer(activity.project.customer) }}
-                            {% if activity.project.customer.teams|length == 0 %}
+                        </td>
-                                {{ widgets.icon('unlocked') }}
+                        <td class="w-min">
-                            {% endif %}
+                            {{ widgets.badge_team_access(activity.project.customer.teams) }}
-                            &nbsp;
+                        </td>
+                        <td class="actions">
                             {{ customerActions.customer(activity.project.customer, 'custom') }}
                         </td>
                     </tr>
@@ -57,10 +58,11 @@
                         <th>{{ 'label.project'|trans }}</th>
                         <td>
                             {{ widgets.label_project(activity.project) }}
-                            {% if activity.project.teams|length == 0 %}
+                        </td>
-                                {{ widgets.icon('unlocked') }}
+                        <td class="w-min">
-                            {% endif %}
+                            {{ widgets.badge_team_access(activity.project.teams) }}
-                            &nbsp;
+                        </td>
+                        <td class="actions">
                             {{ projectActions.project(activity.project, 'custom') }}
                         </td>
                     </tr>
@@ -68,7 +70,7 @@
                 {% for metaField in activity.visibleMetaFields %}
                     <tr>
                         <th>{{ metaField.label|trans }}</th>
-                        <td>{{ widgets.form_type_value(metaField.type, metaField.value, activity) }}</td>
+                        <td colspan="3">{{ widgets.form_type_value(metaField.type, metaField.value, activity) }}</td>
                     </tr>
                 {% endfor %}
             </table>

templates/activity/edit.html.twig

@@ -7,7 +7,7 @@
 {% block main %}
     {% set formEditTemplate = app.request.xmlHttpRequest ? 'default/_form_modal.html.twig' : 'default/_form.html.twig' %}
     {% set formOptions = {
-        'title': activity.name|default('create'|trans),
+        'title': activity.name|default('create-activity'|trans({}, 'actions')),
         'form': form,
         'back': path('admin_activity')
     } %}

templates/customer/actions.html.twig

@@ -48,7 +48,7 @@
             {% set actions = actions|merge({'timesheet': path('admin_timesheet', {'customers[]': customer.id})}) %}
         {% endif %}
         {% if customer.visible and is_granted('create_project') %}
-            {% set actions = actions|merge({'create-project': path('admin_project_create_with_customer', {'customer': customer.id})}) %}
+            {% set actions = actions|merge({'create-project': {'url': path('admin_project_create_with_customer', {'customer': customer.id}), 'class': 'modal-ajax-form'}}) %}
         {% endif %}
         {% if view == 'index' and is_granted('delete', customer) %}
             {% set actions = actions|merge({'trash': {'url': path('admin_customer_delete', {'id': customer.id}), 'class': 'modal-ajax-form'}}) %}

templates/customer/details.html.twig

@@ -27,7 +27,7 @@
                     {{ customer.comment|comment2html(true) }}
                 </div>
             {% endif %}
-            <table class="table table-hover">
+            <table class="table table-hover dataTable">
                 {% if not customer.visible %}
                     <tr>
                         <th>{{ 'label.visible'|trans }}</th>

templates/embeds/comments.html.twig

@@ -13,7 +13,7 @@
         {% endif %}
         {% for comment in comments %}
             <div class="box-comment">
-                {{ widgets.user_avatar(comment.createdBy, false, 'img-sm') }}
+                {{ widgets.user_avatar(comment.createdBy, comment.createdAt|date_full, 'img-sm') }}
                 <div class="comment-text">
                     <span class="username">
                         {{ widgets.username(comment.createdBy) }}
@@ -26,7 +26,6 @@
                             {% if route_delete is not null %}
                                 <a href="{{ path(route_delete, {'id': comment.id}) }}" class="confirmation-link btn btn-default btn-xs" data-question="confirm.delete"><i class="{{ 'delete'|icon }}"></i></a>
                             {% endif %}
-                            <span class="hidden-xs">{{ comment.createdAt|date_full }}</span>
                         </span>
                     </span>
                     {{ comment.message|replace(replacer)|md2html }}

templates/project/actions.html.twig

@@ -46,7 +46,7 @@
             {% set actions = actions|merge({'timesheet': path('admin_timesheet', {'customers[]': project.customer.id, 'projects[]': project.id})}) %}
         {% endif %}
         {% if project.visible and project.customer.visible and is_granted('create_activity') %}
-            {% set actions = actions|merge({'create-activity': path('admin_activity_create_with_project', {'project': project.id})}) %}
+            {% set actions = actions|merge({'create-activity': {'url': path('admin_activity_create_with_project', {'project': project.id}), 'class': 'modal-ajax-form'}}) %}
         {% endif %}
         {% if (view == 'index' or view == 'custom') and is_granted('delete', project) %}
             {% set actions = actions|merge({'trash': {'url': path('admin_project_delete', {'id': project.id}), 'class': 'modal-ajax-form'}}) %}

templates/project/details.html.twig

@@ -27,11 +27,11 @@
                     {{ project.comment|comment2html(true) }}
                 </div>
             {% endif %}
-            <table class="table table-hover">
+            <table class="table table-hover dataTable">
                 {% if not project.visible %}
                     <tr>
                         <th>{{ 'label.visible'|trans }}</th>
-                        <td>
+                        <td colspan="3">
                             {{ widgets.label_boolean(project.visible) }}
                         </td>
                     </tr>
@@ -40,23 +40,24 @@
                     <th>{{ 'label.customer'|trans }}</th>
                     <td>
                         {{ widgets.label_customer(project.customer) }}
-                        {% if project.customer.teams|length == 0 %}
+                    </td>
-                            {{ widgets.icon('unlocked') }}
+                    <td class="w-min">
-                        {% endif %}
+                        {{ widgets.badge_team_access(project.customer.teams) }}
-                        &nbsp;
+                    </td>
+                    <td class="actions">
                         {{ customerActions.customer(project.customer, 'custom') }}
                     </td>
                 </tr>
                 {% if is_granted('details', project) %}
                     <tr>
                         <th>{{ 'label.orderNumber'|trans }}</th>
-                        <td>
+                        <td colspan="3">
                             {{ project.orderNumber }}
                         </td>
                     </tr>
                     <tr>
                         <th>{{ 'label.orderDate'|trans }}</th>
-                        <td>
+                        <td colspan="3">
                             {% if project.orderDate is not empty %}
                                 {{ project.orderDate|date_full }}
                             {% else %}
@@ -66,7 +67,7 @@
                     </tr>
                     <tr>
                         <th>{{ 'label.project_start'|trans }}</th>
-                        <td>
+                        <td colspan="3">
                             {% if project.start is not empty %}
                                 {{ project.start|date_full }}
                             {% else %}
@@ -76,7 +77,7 @@
                     </tr>
                     <tr>
                         <th>{{ 'label.project_end'|trans }}</th>
-                        <td>
+                        <td colspan="3">
                             {% if project.end is not empty %}
                                 {{ project.end|date_full }}
                             {% else %}
@@ -88,7 +89,7 @@
                 {% for metaField in project.visibleMetaFields %}
                     <tr>
                         <th>{{ metaField.label|trans }}</th>
-                        <td>{{ widgets.form_type_value(metaField.type, metaField.value, project) }}</td>
+                        <td colspan="3">{{ widgets.form_type_value(metaField.type, metaField.value, project) }}</td>
                     </tr>
                 {% endfor %}
             </table>

templates/project/edit.html.twig

@@ -7,7 +7,7 @@
 {% block main %}
     {% set formEditTemplate = app.request.xmlHttpRequest ? 'default/_form_modal.html.twig' : 'default/_form.html.twig' %}
     {% set formOptions = {
-        'title': project.name|default('create'|trans),
+        'title': project.name|default('create-project'|trans({}, 'actions')),
         'form': form,
         'back': path('admin_project')
     } %}

templates/user/form.html.twig

@@ -1,15 +1,20 @@
 {% extends 'user/layout.html.twig' %}
 
 {% block main %}
-
+    {{ form_start(form) }}
-    {% embed '@AdminLTE/Widgets/box-widget.html.twig' %}
+    {% for section, counter in sections %}
-        {% block box_title %}{{ ('profile.' ~ tab)|trans }}{% endblock %}
+        {% embed '@AdminLTE/Widgets/box-widget.html.twig' %}
-        {% block box_body %}
+            {% block box_body %}
-            {{ form_start(form) }}
+                {% for pref in form.children.preferences %}
-            {{ form_widget(form) }}
+                    {% if pref.vars.data.section == section %}
-            <input type="submit" value="{{ 'action.save'|trans }}" class="btn btn-primary" />
+                        {{ form_row(pref) }}
-            {{ form_end(form) }}
+                    {% endif %}
-        {% endblock %}
+                {% endfor %}
-    {% endembed %}
+            {% endblock %}
+        {% endembed %}
+    {% endfor %}
+    {{ form_widget(form) }}
+    <input type="submit" value="{{ 'action.save'|trans }}" class="btn btn-primary" />
+    {{ form_end(form) }}
 
 {% endblock %}

tests/Controller/ProjectControllerTest.php

@@ -299,6 +299,7 @@ class ProjectControllerTest extends ControllerBaseTest
         $client->submit($form, [
             'project_edit_form' => [
                 'name' => 'Test 2',
+                'customer' => 1,
             ]
         ]);
         $this->assertIsRedirect($client, $this->createUrl('/admin/project/2/details'));

tests/Entity/UserPreferenceTest.php

@@ -0,0 +1,63 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Tests\Entity;
+
+use App\Entity\UserPreference;
+use App\Form\Type\YesNoType;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Form\Extension\Core\Type\CheckboxType;
+use Symfony\Component\Form\Extension\Core\Type\IntegerType;
+
+/**
+ * @covers \App\Entity\UserPreference
+ */
+class UserPreferenceTest extends TestCase
+{
+    public function testDefaultValues()
+    {
+        $sut = new UserPreference();
+        self::assertTrue($sut->isEnabled());
+        self::assertEquals('default', $sut->getSection());
+        self::assertEquals(1000, $sut->getOrder());
+        self::assertNull($sut->getValue());
+        self::assertIsArray($sut->getConstraints());
+        self::assertEmpty($sut->getConstraints());
+        self::assertNull($sut->getId());
+        self::assertNull($sut->getLabel());
+        self::assertNull($sut->getName());
+        self::assertIsArray($sut->getOptions());
+        self::assertEmpty($sut->getOptions());
+        self::assertNull($sut->getType());
+        self::assertNull($sut->getUser());
+    }
+
+    public function testGetValueChangesReturnTypeOnOtherType()
+    {
+        $sut = new UserPreference();
+        $sut->setValue('1');
+        self::assertSame('1', $sut->getValue());
+        $sut->setType(IntegerType::class);
+        self::assertSame(1, $sut->getValue());
+        $sut->setType(YesNoType::class);
+        self::assertSame(true, $sut->getValue());
+        $sut->setValue('0');
+        $sut->setType(CheckboxType::class);
+        self::assertSame(false, $sut->getValue());
+    }
+
+    public function testGetLabelWithLabelOption()
+    {
+        $sut = new UserPreference();
+        $sut->setName('foo');
+        self::assertEquals('foo', $sut->getLabel());
+        $sut->setOptions(['label' => 'bar']);
+        self::assertEquals('bar', $sut->getLabel());
+    }
+}