Reviewed tag in seach work

- Refactored some tag code bits while reviewing.
- Updated tag design in search listing to be more subtle.
- Moved tags out of entity-list-item-basic template and instead moved
  them into entity-list-item, below the existing content.
- Tweaked existing tag colors a little.
- Changed tag icon to be more tag-like.
- Added tag-on-search test case.

Review of #2487, Related to #2462

app/Actions/Tag.php

@@ -1,6 +1,7 @@
 <?php namespace BookStack\Actions;
 
 use BookStack\Model;
+use Illuminate\Database\Eloquent\Relations\MorphTo;
 
 class Tag extends Model
 {
@@ -9,10 +10,25 @@ class Tag extends Model
 
     /**
      * Get the entity that this tag belongs to
-     * @return \Illuminate\Database\Eloquent\Relations\MorphTo
      */
-    public function entity()
+    public function entity(): MorphTo
     {
         return $this->morphTo('entity');
     }
+
+    /**
+     * Get a full URL to start a tag name search for this tag name.
+     */
+    public function nameUrl(): string
+    {
+        return url('/search?term=%5B' . urlencode($this->name) .'%5D');
+    }
+
+    /**
+     * Get a full URL to start a tag name and value search for this tag's values.
+     */
+    public function valueUrl(): string
+    {
+        return url('/search?term=%5B' . urlencode($this->name) .'%3D' . urlencode($this->value) . '%5D');
+    }
 }

resources/icons/tag.svg

@@ -1,4 +1 @@
-<svg viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg">
+<svg xmlns="http://www.w3.org/2000/svg" enable-background="new 0 0 24 24" viewBox="0 0 24 24"><path d="M21.41,11.41l-8.83-8.83C12.21,2.21,11.7,2,11.17,2H4C2.9,2,2,2.9,2,4v7.17c0,0.53,0.21,1.04,0.59,1.41l8.83,8.83 c0.78,0.78,2.05,0.78,2.83,0l7.17-7.17C22.2,13.46,22.2,12.2,21.41,11.41z M6.5,8C5.67,8,5,7.33,5,6.5S5.67,5,6.5,5S8,5.67,8,6.5 S7.33,8,6.5,8z"/></svg>
-    <path d="M0 0h24v24H0z" fill="none"/>
-    <path d="M17.63 5.84C17.27 5.33 16.67 5 16 5L5 5.01C3.9 5.01 3 5.9 3 7v10c0 1.1.9 1.99 2 1.99L16 19c.67 0 1.27-.33 1.63-.84L22 12l-4.37-6.16z"/>
-</svg>

resources/sass/_blocks.scss

@@ -224,12 +224,13 @@
   margin-bottom: $-xs;
   margin-inline-end: $-xs;
   border-radius: 4px;
-  border: 1px solid #CCC;
+  border: 1px solid;
   overflow: hidden;
   font-size: 0.85em;
+  @include lightDark(border-color, #CCC, #666);
   a, span, a:hover, a:active {
     padding: 4px 8px;
-    @include lightDark(color, #777, #999);
+    @include lightDark(color, rgba(0, 0, 0, 0.6), rgba(255, 255, 255, 0.8));
     transition: background-color ease-in-out 80ms;
     text-decoration: none;
   }
@@ -237,10 +238,11 @@
     @include lightDark(background-color, rgba(255, 255, 255, 0.7), rgba(255, 255, 255, 0.3));
   }
   svg {
-    fill: #888;
+    @include lightDark(fill, rgba(0, 0, 0, 0.5), rgba(255, 255, 255, 0.5));
   }
   .tag-value {
-    border-inline-start: 1px solid #DDD;
+    border-inline-start: 1px solid;
+    @include lightDark(border-color, #DDD, #666);
     @include lightDark(background-color, rgba(255, 255, 255, 0.5), rgba(255, 255, 255, 0.2))
   }
 }

resources/sass/_lists.scss

@@ -550,6 +550,17 @@ ul.pagination {
   }
 }
 
+.entity-item-tags {
+  font-size: .75rem;
+  opacity: 1;
+  .primary-background-light {
+    background: transparent;
+  }
+  .tag-name {
+    background-color: rgba(0, 0, 0, 0.05);
+  }
+}
+
 .dropdown-container {
   display: inline-block;
   vertical-align: top;

resources/views/components/tag-list.blade.php

@@ -1,11 +1,11 @@
 @foreach($entity->tags as $tag)
     <div class="tag-item primary-background-light">
-        @if($disableLinks ?? false)
+        @if($linked ?? true)
+            <div class="tag-name"><a href="{{ $tag->nameUrl() }}">@icon('tag'){{ $tag->name }}</a></div>
+            @if($tag->value) <div class="tag-value"><a href="{{ $tag->valueUrl() }}">{{$tag->value}}</a></div> @endif
+        @else
             <div class="tag-name"><span>@icon('tag'){{ $tag->name }}</span></div>
             @if($tag->value) <div class="tag-value"><span>{{$tag->value}}</span></div> @endif
-        @else
-            <div class="tag-name"><a href="{{ url('/search?term=%5B' . urlencode($tag->name) .'%5D') }}">@icon('tag'){{ $tag->name }}</a></div>
-            @if($tag->value) <div class="tag-value"><a href="{{ url('/search?term=%5B' . urlencode($tag->name) .'%3D' . urlencode($tag->value) . '%5D') }}">{{$tag->value}}</a></div> @endif
         @endif
     </div>
 @endforeach

resources/views/partials/entity-list-item-basic.blade.php

@@ -2,9 +2,6 @@
 <a href="{{ $entity->getUrl() }}" class="{{$type}} {{$type === 'page' && $entity->draft ? 'draft' : ''}} {{$classes ?? ''}} entity-list-item" data-entity-type="{{$type}}" data-entity-id="{{$entity->id}}">
     <span role="presentation" class="icon text-{{$type}}">@icon($type)</span>
     <div class="content">
-            @if($entity->tags->count() > 0 and $showTags ?? false)
-                    @include('components.tag-list', ['entity' => $entity, 'disableLinks' => True ])
-            @endif
             <h4 class="entity-list-item-name break-text">{{ $entity->name }}</h4>
             {{ $slot ?? '' }}
     </div>

resources/views/partials/entity-list-item.blade.php

@@ -1,4 +1,5 @@
-@component('partials.entity-list-item-basic', ['entity' => $entity, 'showTags' => $showTags])
+@component('partials.entity-list-item-basic', ['entity' => $entity])
+
 <div class="entity-item-snippet">
 
     @if($showPath ?? false)
@@ -12,4 +13,11 @@
 
     <p class="text-muted break-text">{{ $entity->getExcerpt() }}</p>
 </div>
+
+@if(($showTags ?? false) && $entity->tags->count() > 0)
+    <div class="entity-item-tags mt-xs">
+        @include('components.tag-list', ['entity' => $entity, 'linked' => false ])
+    </div>
+@endif
+
 @endcomponent

tests/Entity/TagTest.php

@@ -1,28 +1,23 @@
 <?php namespace Tests\Entity;
 
-use BookStack\Entities\Models\Book;
-use BookStack\Entities\Models\Chapter;
 use BookStack\Actions\Tag;
 use BookStack\Entities\Models\Entity;
 use BookStack\Entities\Models\Page;
-use BookStack\Auth\Permissions\PermissionService;
+use Tests\TestCase;
-use Tests\BrowserKitTest;
 
-class TagTest extends BrowserKitTest
+class TagTest extends TestCase
 {
 
     protected $defaultTagCount = 20;
 
     /**
      * Get an instance of a page that has many tags.
-     * @param \BookStack\Actions\Tag[]|bool $tags
-     * @return Entity
      */
-    protected function getEntityWithTags($class, $tags = false): Entity
+    protected function getEntityWithTags($class, ?array $tags = null): Entity
     {
         $entity = $class::first();
 
-        if (!$tags) {
+        if (is_null($tags)) {
             $tags = factory(Tag::class, $this->defaultTagCount)->make();
         }
 
@@ -68,8 +63,6 @@ class TagTest extends BrowserKitTest
 
     public function test_entity_permissions_effect_tag_suggestions()
     {
-        $permissionService = $this->app->make(PermissionService::class);
-
         // Create some tags with similar names to test with and save to a page
         $attrs = collect();
         $attrs = $attrs->merge(factory(Tag::class, 5)->make(['name' => 'country']));
@@ -88,4 +81,20 @@ class TagTest extends BrowserKitTest
         $this->asEditor()->get('/ajax/tags/suggest/names?search=co')->seeJsonEquals([]);
     }
 
+    public function test_tags_shown_on_search_listing()
+    {
+        $tags = [
+            factory(Tag::class)->make(['name' => 'category', 'value' => 'buckets']),
+            factory(Tag::class)->make(['name' => 'color', 'value' => 'red']),
+        ];
+
+        $page = $this->getEntityWithTags(Page::class, $tags);
+        $resp = $this->asEditor()->get("/search?term=[category]");
+        $resp->assertSee($page->name);
+        $resp->assertElementContains('[href="' . $page->getUrl() . '"]', 'category');
+        $resp->assertElementContains('[href="' . $page->getUrl() . '"]', 'buckets');
+        $resp->assertElementContains('[href="' . $page->getUrl() . '"]', 'color');
+        $resp->assertElementContains('[href="' . $page->getUrl() . '"]', 'red');
+    }
+
 }