From ec0c972d348ffd5897f115d201031b704875b56a Mon Sep 17 00:00:00 2001
From: undergroundwires <undergroundwires@users.noreply.github.com>
Date: Thu, 26 Aug 2021 21:08:38 +0100
Subject: [PATCH] Fix excessive highlighting on hover

It fixes whitespace on left when being highlighted when hovering on macOS (OS selection button on top)
The commit also unifies the way top menu buttons are displayed by reusing `MenuOptionListItem`s (renamed from `SelectableOption`) and `MenuOptionList`. This ensures right and consistent behavior.
Finally it fixes `enabled` property in menu option setting disabled state instead.
---
 .../Scripts/Menu/Grouping/TheGrouper.vue      | 58 +++++------
 .../Scripts/Menu/MenuOptionList.vue           | 38 ++++++++
 ...tableOption.vue => MenuOptionListItem.vue} | 12 ++-
 .../Scripts/Menu/Selector/TheSelector.vue     | 97 +++++++------------
 .../components/Scripts/Menu/TheOsChanger.vue  | 58 ++++-------
 5 files changed, 122 insertions(+), 141 deletions(-)
 create mode 100644 src/presentation/components/Scripts/Menu/MenuOptionList.vue
 rename src/presentation/components/Scripts/Menu/{Selector/SelectableOption.vue => MenuOptionListItem.vue} (64%)

diff --git a/src/presentation/components/Scripts/Menu/Grouping/TheGrouper.vue b/src/presentation/components/Scripts/Menu/Grouping/TheGrouper.vue
index e77e24dd..23c44704 100644
--- a/src/presentation/components/Scripts/Menu/Grouping/TheGrouper.vue
+++ b/src/presentation/components/Scripts/Menu/Grouping/TheGrouper.vue
@@ -1,26 +1,38 @@
 <template>
     <div class="container">
-        <span class="part">Group by:</span>
+        <MenuOptionList
+            label="Group by"
+            class="part">
+            <MenuOptionListItem
+                label="Cards"
+                :enabled="!cardsSelected"
+                @click="groupByCard()"
+            />
+            <MenuOptionListItem
+                label="None"
+                :enabled="!noneSelected"
+                @click="groupByNone()"
+            />
+        </MenuOptionList>
         <span class="part">
-            <span 
-                class="part"
-                v-bind:class="{ 'disabled': cardsSelected, 'enabled': !cardsSelected}"
-                @click="groupByCard()">Cards</span>
-            <span class="part">|</span>
-            <span class="part"
-                v-bind:class="{ 'disabled': noneSelected, 'enabled': !noneSelected}"
-                @click="groupByNone()">None</span>
-            </span>
+        </span>
     </div>
 </template>
 
 <script lang="ts">
 import { Component, Vue } from 'vue-property-decorator';
 import { Grouping } from './Grouping';
+import MenuOptionList from './../MenuOptionList.vue';
+import MenuOptionListItem from './../MenuOptionListItem.vue';
 
 const DefaultGrouping = Grouping.Cards;
 
-@Component
+@Component({
+  components: {
+    MenuOptionList,
+    MenuOptionListItem,
+  },
+})
 export default class TheGrouper extends Vue {
     public cardsSelected = false;
     public noneSelected = false;
@@ -50,29 +62,5 @@ export default class TheGrouper extends Vue {
 </script>
 
 <style scoped lang="scss">
-@import "@/presentation/styles/colors.scss";
-@import "@/presentation/styles/fonts.scss";
-
-.container {
-    display: flex;
-    flex-direction: row;
-    flex-wrap: wrap;
-    font-family: $normal-font;
-    .part {
-        display: flex;
-        margin-right:5px;
-    }
-}
-
-.enabled {
-    cursor: pointer;
-    &:hover {
-        font-weight:bold;
-        text-decoration:underline;
-    }
-}
-.disabled {
-    color:$gray;
-}
 
 </style>
diff --git a/src/presentation/components/Scripts/Menu/MenuOptionList.vue b/src/presentation/components/Scripts/Menu/MenuOptionList.vue
new file mode 100644
index 00000000..135bbbae
--- /dev/null
+++ b/src/presentation/components/Scripts/Menu/MenuOptionList.vue
@@ -0,0 +1,38 @@
+<template>
+    <div class="list">
+        <div v-if="label">{{ label }}:</div>
+        <div class="items">
+            <slot />
+        </div>
+    </div>
+</template>
+
+<script lang="ts">
+import { Component, Vue, Prop } from 'vue-property-decorator';
+
+@Component
+export default class MenuOptionList extends Vue {
+  @Prop() public label: string;
+}
+</script>
+
+<style scoped lang="scss">
+@import "@/presentation/styles/fonts.scss";
+
+$gap: 0.25rem;
+.list {
+  font-family: $normal-font;
+  display: flex;
+  align-items: center;
+  .items {
+    * + *::before {
+        content: '|';
+        padding-right: $gap;
+        padding-left: $gap;
+    }
+  }
+  > *:not(:last-child) {
+    margin-right: $gap;
+  }
+}
+</style>
\ No newline at end of file
diff --git a/src/presentation/components/Scripts/Menu/Selector/SelectableOption.vue b/src/presentation/components/Scripts/Menu/MenuOptionListItem.vue
similarity index 64%
rename from src/presentation/components/Scripts/Menu/Selector/SelectableOption.vue
rename to src/presentation/components/Scripts/Menu/MenuOptionListItem.vue
index 02b96b3f..1c98c9ce 100644
--- a/src/presentation/components/Scripts/Menu/Selector/SelectableOption.vue
+++ b/src/presentation/components/Scripts/Menu/MenuOptionListItem.vue
@@ -1,8 +1,10 @@
 <template>
-    <span
-        v-bind:class="{ 'disabled': enabled, 'enabled': !enabled}"
-        v-non-collapsing
-        @click="!enabled && onClicked()">{{label}}</span>
+    <span> <!-- Parent wrapper allows adding content inside with CSS without making it clickable -->
+        <span
+            v-bind:class="{ 'disabled': !enabled, 'enabled': enabled}"
+            v-non-collapsing
+            @click="enabled && onClicked()">{{label}}</span>
+    </span>
 </template>
 
 <script lang="ts">
@@ -12,7 +14,7 @@ import { NonCollapsing } from '@/presentation/components/Scripts/Cards/NonCollap
 @Component({
     directives: { NonCollapsing },
 })
-export default class SelectableOption extends Vue {
+export default class MenuOptionListItem extends Vue {
   @Prop() public enabled: boolean;
   @Prop() public label: string;
   @Emit('click') public onClicked() { return; }
diff --git a/src/presentation/components/Scripts/Menu/Selector/TheSelector.vue b/src/presentation/components/Scripts/Menu/Selector/TheSelector.vue
index 3e396648..2053c220 100644
--- a/src/presentation/components/Scripts/Menu/Selector/TheSelector.vue
+++ b/src/presentation/components/Scripts/Menu/Selector/TheSelector.vue
@@ -1,63 +1,51 @@
 <template>
-    <div class="container">
-        <div class="part">Select:</div>
-        <div class="part">
-            <div class="part">
-                <SelectableOption
-                    label="None"
-                    :enabled="this.currentSelection == SelectionType.None"
-                    @click="selectType(SelectionType.None)"
-                    v-tooltip=" 'Deselect all selected scripts.<br/>' +
-                                '💡 Good start to dive deeper into tweaks and select only what you want.'"
-                    />
-            </div>
-            <div class="part"> | </div>
-            <div class="part">
-                <SelectableOption
-                    label="Standard"
-                    :enabled="this.currentSelection == SelectionType.Standard"
-                    @click="selectType(SelectionType.Standard)"
-                    v-tooltip=" '🛡️ Balanced for privacy and functionality.<br/>' +
-                                'OS and applications will function normally.<br/>' +
-                                '💡 Recommended for everyone'"
-                    />
-                </div>
-            <div class="part"> | </div>
-            <div class="part">
-                <SelectableOption
-                    label="Strict"
-                    :enabled="this.currentSelection == SelectionType.Strict"
-                    @click="selectType(SelectionType.Strict)"
-                    v-tooltip=" '🚫 Stronger privacy, disables risky functions that may leak your data.<br/>' +
-                                '⚠️ Double check to remove scripts where you would trade functionality for privacy<br/>' +
-                                '💡 Recommended for daily users that prefers more privacy over non-essential functions'"
-                    />
-                </div>
-            <div class="part"> | </div>
-            <div class="part">
-                    <SelectableOption
-                        label="All"
-                        :enabled="this.currentSelection == SelectionType.All"
-                        @click="selectType(SelectionType.All)"
-                        v-tooltip=" '🔒 Strongest privacy, disabling any functionality that may leak your data.<br/>' +
-                                    '🛑 Not designed for daily users, it will break important functionalities.<br/>' +
-                                    '💡 Only recommended for extreme use-cases like crime labs where no leak is acceptable'"
-                    />
-            </div>
-        </div>
-    </div>
+    <MenuOptionList label="Select">
+        <MenuOptionListItem
+            label="None"
+            :enabled="this.currentSelection !== SelectionType.None"
+            @click="selectType(SelectionType.None)"
+            v-tooltip=" 'Deselect all selected scripts.<br/>' +
+                        '💡 Good start to dive deeper into tweaks and select only what you want.'"
+            />
+        <MenuOptionListItem
+            label="Standard"
+            :enabled="this.currentSelection !== SelectionType.Standard"
+            @click="selectType(SelectionType.Standard)"
+            v-tooltip=" '🛡️ Balanced for privacy and functionality.<br/>' +
+                        'OS and applications will function normally.<br/>' +
+                        '💡 Recommended for everyone'"
+            />
+        <MenuOptionListItem
+            label="Strict"
+            :enabled="this.currentSelection !== SelectionType.Strict"
+            @click="selectType(SelectionType.Strict)"
+            v-tooltip=" '🚫 Stronger privacy, disables risky functions that may leak your data.<br/>' +
+                        '⚠️ Double check to remove scripts where you would trade functionality for privacy<br/>' +
+                        '💡 Recommended for daily users that prefers more privacy over non-essential functions'"
+            />
+        <MenuOptionListItem
+            label="All"
+            :enabled="this.currentSelection !== SelectionType.All"
+            @click="selectType(SelectionType.All)"
+            v-tooltip=" '🔒 Strongest privacy, disabling any functionality that may leak your data.<br/>' +
+                        '🛑 Not designed for daily users, it will break important functionalities.<br/>' +
+                        '💡 Only recommended for extreme use-cases like crime labs where no leak is acceptable'"
+        />
+    </MenuOptionList>
 </template>
 
 <script lang="ts">
 import { Component } from 'vue-property-decorator';
 import { StatefulVue } from '@/presentation/components/Shared/StatefulVue';
-import SelectableOption from './SelectableOption.vue';
 import { ICategoryCollectionState } from '@/application/Context/State/ICategoryCollectionState';
 import { SelectionType, SelectionTypeHandler } from './SelectionTypeHandler';
+import MenuOptionList from './../MenuOptionList.vue';
+import MenuOptionListItem from '../MenuOptionListItem.vue';
 
 @Component({
   components: {
-    SelectableOption,
+    MenuOptionList,
+    MenuOptionListItem,
   },
 })
 export default class TheSelector extends StatefulVue {
@@ -87,16 +75,5 @@ export default class TheSelector extends StatefulVue {
 </script>
 
 <style scoped lang="scss">
-@import "@/presentation/styles/fonts.scss";
 
-.container {
-    display: flex;
-    flex-direction: row;
-    flex-wrap: wrap;
-    .part {
-        display: flex;
-        margin-right:5px;
-    }
-    font-family: $normal-font;
-}
 </style>
diff --git a/src/presentation/components/Scripts/Menu/TheOsChanger.vue b/src/presentation/components/Scripts/Menu/TheOsChanger.vue
index 2997060f..3150b481 100644
--- a/src/presentation/components/Scripts/Menu/TheOsChanger.vue
+++ b/src/presentation/components/Scripts/Menu/TheOsChanger.vue
@@ -1,17 +1,12 @@
 <template>
-  <div class="container">
-    <!-- <div>OS:</div> -->
-    <div class="os-list">
-      <div v-for="os in this.allOses" :key="os.name">
-        <span
-          class="os-name"
-          v-bind:class="{ 'current': currentOs === os.os }"
-          v-on:click="changeOsAsync(os.os)">
-          {{ os.name }}
-        </span>
-      </div>
-    </div>
-  </div>
+  <MenuOptionList>
+      <MenuOptionListItem
+        v-for="os in this.allOses" :key="os.name"
+        :enabled="currentOs !== os.os"
+        @click="changeOsAsync(os.os)"
+        :label="os.name"
+        />
+  </MenuOptionList>
 </template>
 
 <script lang="ts">
@@ -20,8 +15,15 @@ import { OperatingSystem } from '@/domain/OperatingSystem';
 import { StatefulVue } from '@/presentation/components/Shared/StatefulVue';
 import { ICategoryCollectionState } from '@/application/Context/State/ICategoryCollectionState';
 import { ApplicationFactory } from '@/application/ApplicationFactory';
+import MenuOptionList from './MenuOptionList.vue';
+import MenuOptionListItem from './MenuOptionListItem.vue';
 
-@Component
+@Component({
+  components: {
+    MenuOptionList,
+    MenuOptionListItem,
+  },
+})
 export default class TheOsChanger extends StatefulVue {
   public allOses: Array<{ name: string, os: OperatingSystem }> = [];
   public currentOs?: OperatingSystem = null;
@@ -52,31 +54,5 @@ function renderOsName(os: OperatingSystem): string {
 </script>
 
 <style scoped lang="scss">
-@import "@/presentation/styles/fonts.scss";
-@import "@/presentation/styles/colors.scss";
-.container {
-  font-family: $normal-font;
-  display: flex;
-  align-items: center;
-  .os-list {
-    display: flex;
-    margin-left: 0.25rem;
-    div + div::before {
-        content: "|";
-        margin-left: 0.5rem;
-    }
-    .os-name {
-      &:not(.current) {
-        cursor: pointer;
-        &:hover {
-            font-weight: bold;
-            text-decoration: underline;
-        }
-      }
-      &.current {
-        color: $gray;
-      }
-    }
-  }
-}
+
 </style>