Repository: manusa/electronim
Pull request #91 (2022-09-03 06:41:17)

Feat/ux

sonarcloud[bot]: Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=manusa_electronim&pullRequest=91)

[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=91&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=91&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=91&resolved=false&types=BUG)  
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=91&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=91&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=91&resolved=false&types=VULNERABILITY)  
[![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=manusa_electronim&pullRequest=91&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=manusa_electronim&pullRequest=91&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=manusa_electronim&pullRequest=91&resolved=false&types=SECURITY_HOTSPOT)  
[![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=91&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=91&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=91&resolved=false&types=CODE_SMELL)

[![91.2%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '91.2%')](https://sonarcloud.io/component_measures?id=manusa_electronim&pullRequest=91&metric=new_coverage&view=list) [91.2% Coverage](https://sonarcloud.io/component_measures?id=manusa_electronim&pullRequest=91&metric=new_coverage&view=list)  
[![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=manusa_electronim&pullRequest=91&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=manusa_electronim&pullRequest=91&metric=new_duplicated_lines_density&view=list)


sonarcloud[bot]: Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=manusa_electronim&pullRequest=91)

[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=91&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=91&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=91&resolved=false&types=BUG)  
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=91&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=91&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=91&resolved=false&types=VULNERABILITY)  
[![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=manusa_electronim&pullRequest=91&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=manusa_electronim&pullRequest=91&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=manusa_electronim&pullRequest=91&resolved=false&types=SECURITY_HOTSPOT)  
[![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=91&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=91&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=91&resolved=false&types=CODE_SMELL)

[![91.4%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '91.4%')](https://sonarcloud.io/component_measures?id=manusa_electronim&pullRequest=91&metric=new_coverage&view=list) [91.4% Coverage](https://sonarcloud.io/component_measures?id=manusa_electronim&pullRequest=91&metric=new_coverage&view=list)  
[![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=manusa_electronim&pullRequest=91&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=manusa_electronim&pullRequest=91&metric=new_duplicated_lines_density&view=list)



Patch:
diff --git a/src/components/README.md b/src/components/README.md
new file mode 100644
index 0000000..8c1b4c4
--- /dev/null
+++ b/src/components/README.md
@@ -0,0 +1,33 @@
+# Components
+
+This module contains the library of shared components to be used accross the application.
+
+## Structure
+
+Since ElectronIM is based on Preact in combination with Electron, to be able to use the component within a browser, you need to pass the `html` bound `h` function from `preact` to the component.
+
+Every component definition will start with a function that accepts the `html` parameter that returns an arrow function with the component.
+
+```javascript
+module.exports.Component = html => ({property1}) => html`
+  <div>Your Component Definition</div>
+`;
+```
+
+## Using Components
+
+Components need to be imported in a `preload.js` script in order to be used ina `browser-xxx.js` file.
+
+```javascript
+window.preact = require('preact');
+window.html = require('htm').bind(window.preact.h);
+window.Component = require('../components').component(window.html);
+```
+
+Then you can use the component in the `browser-xxx.js` file.
+
+```javascript
+const {html, Component, preact: {render}} = window;
+const App = () => html`<${Component} property1='value1' />`;
+render(html`<${App} />`, document.querySelector('.root'));
+```
diff --git a/src/components/index.js b/src/components/index.js
new file mode 100644
index 0000000..91c80db
--- /dev/null
+++ b/src/components/index.js
@@ -0,0 +1,3 @@
+module.exports = {
+  topBar: require('./top-bar').topBar
+};
diff --git a/src/components/top-bar.js b/src/components/top-bar.js
new file mode 100644
index 0000000..e88e388
--- /dev/null
+++ b/src/components/top-bar.js
@@ -0,0 +1,23 @@
+/**
+ * A TopBar partially based on Bulma's Navbar component.
+ */
+module.exports.topBar = html => ({
+  title,
+  titleClass = 'navbar-item is-size-5 has-text-info has-text-weight-bold',
+  containerClass = '',
+  endComponents = '',
+  endComponentsClass = '',
+  fixed = false
+}) => {
+  if (fixed) {
+    document.body.classList.add('has-navbar-fixed-top');
+  }
+  return html`
+    <nav class=${`navbar ${fixed ? 'is-fixed-top' : ''}`}>
+      <div class=${`not-navbar-brand is-flex is-flex-grow-1 is-align-content-center ${containerClass}`}>
+        <div class=${`not-navbar-brand is-flex is-flex-grow-1 ${titleClass}`}>${title}</div>
+        <div class=${endComponentsClass}>${endComponents}</div>
+      </div>
+    </nav>
+  `;
+};
diff --git a/src/settings/browser-settings.js b/src/settings/browser-settings.js
index cad411a..738e603 100644
--- a/src/settings/browser-settings.js
+++ b/src/settings/browser-settings.js
@@ -14,9 +14,8 @@
    limitations under the License.
  */
 /* eslint-disable no-undef */
-const {h, render} = window.preact;
-const {useReducer} = window.preactHooks;
-const html = window.htm.bind(h);
+const {html, preact: {render}, preactHooks: {useReducer}, TopBar} = window;
+
 const settings = () => document.querySelector('.settings');
 
 const prependProtocol = url => {
@@ -26,13 +25,17 @@ const prependProtocol = url => {
   return url;
 };
 
-const validateUrl = url => {
-  url = prependProtocol(url);
+const validateUrl = (url, allowNoProtocol = true) => {
+  if (allowNoProtocol) {
+    url = prependProtocol(url);
+  }
   if (!url && !url.match(/^https?:\/\/.+/)) {
     return false;
   }
   try {
-    return new URL(url);
+    // eslint-disable-next-line no-unused-vars
+    const ignored = new URL(url);
+    return true;
   } catch (error) {
     /* error is ignored */
   }
@@ -56,6 +59,7 @@ const initialState = {
   expandedTabs: [],
   newTabValid: false,
   newTabValue: '',
+  invalidTabs: new Set(),
   canSave: window.tabs.length > 0,
   disableNotificationsGlobally: window.disableNotificationsGlobally
 };
@@ -63,6 +67,7 @@ const initialState = {
 const ACTIONS = {
   ADD: 'ADD',
   REMOVE: 'REMOVE',
+  SET_TAB_PROPERTY: 'SET_TAB_PROPERTY',
   TOGGLE_DICTIONARY: 'TOGGLE_DICTIONARY',
   TOGGLE_TAB_EXPANDED: 'TOGGLE_TAB_EXPANDED',
   TOGGLE_TAB_PROPERTY: 'TOGGLE_TAB_PROPERTY',
@@ -84,6 +89,7 @@ const reducer = (state, action) => {
         newTabValue: '',
         tabs: [...state.tabs, {
           id: newId(),
+          disabled: false,
           sandboxed: false,
           disableNotifications: false,
           url: prependProtocol(state.newTabValue)
@@ -97,6 +103,22 @@ const reducer = (state, action) => {
         canSave: state.tabs.filter(tab => tab.id !== action.payload.id).length > 0
       };
     }
+    case ACTIONS.SET_TAB_PROPERTY: {
+      const newState = {...state, tabs: []};
+      state.tabs.forEach(tab => {
+        const newTab = {...tab};
+        if (newTab.id === action.payload.id) {
+          newTab[action.payload.property] = action.payload.value;
+          if (!validateUrl(newTab.url, false)) {
+            newState.invalidTabs.add(newTab.id);
+          } else {
+            newState.invalidTabs.delete(newTab.id);
+          }
+        }
+        newState.tabs.push(newTab);
+      });
+      return newState;
+    }
     case ACTIONS.TOGGLE_DICTIONARY: {
       const newState = {...state};
       if (dictionariesEnabled(newState).includes(action.payload)) {
@@ -143,6 +165,8 @@ const reducer = (state, action) => {
   }
 };
 
+const setTabProperty = ({dispatch, property, value, id}) =>
+  dispatch({type: ACTIONS.SET_TAB_PROPERTY, payload: {id, property, value}});
 const toggleTabProperty = (dispatch, property, id) =>
   () => dispatch({type: ACTIONS.TOGGLE_TAB_PROPERTY, payload: {id, property}});
 
@@ -174,29 +198,36 @@ const Checkbox = ({label, title = '', icon, checked, value, onclick}) => (html`
   </div>
 `);
 
+const disabledIcon = disabled => (disabled === true ? 'fa-eye-slash' : 'fa-eye');
+const sandboxedIcon = sandboxed => (sandboxed === true ? 'fa-lock' : 'fa-lock-open');
+
 const TabAdvancedSettings = (
-  {dispatch, id, disabled = false, sandboxed = false}
+  {dispatch, id, sandboxed = false}
 ) => (html`
   <div class="settings__tab-advanced container">
-    <${Checkbox} label="Disable" checked=${disabled} value=${id}
-      icon=${disabled ? 'fa-eye-slash' : 'fa-eye'}
-      onclick=${toggleTabProperty(dispatch, 'disabled', id)}
-    />
     <${Checkbox} label="Sandbox" checked=${sandboxed} value=${id}
       title='Use an isolated/sandboxed session for this tab'
-      icon=${sandboxed ? 'fa-lock' : 'fa-lock-open'}
+      icon=${sandboxedIcon(sandboxed)}
       onclick=${toggleTabProperty(dispatch, 'sandboxed', id)}
     />
   </div>
 `);
 
-const TabEntry = ({dispatch, id, expanded, url, disableNotifications = false, ...tab}) => (html`
+const TabEntry = ({
+  dispatch, invalidTabs, id, expanded, url, disabled, disableNotifications = false, ...tab
+}) => (html`
   <div class='settings__tab ${expanded && 'settings__tab--expanded'} panel-block' data-id=${id}>
     <div class='settings__tab-main'>
       <${ExpandButton} dispatch=${dispatch} id=${id} expanded=${expanded} />
       <div class='control'>
-        <input type='text' readonly class='input' name='tabs' value='${url}' />
+        <input type='text' class=${`input ${invalidTabs.has(id) ? 'is-danger' : ''}`} name='tabs'
+          oninput=${({target: {value}}) => setTabProperty({dispatch, property: 'url', value, id})}
+          value=${url} />
       </div>
+      <${SettingsButton} icon=${disabledIcon(disabled)}
+        title=${disabled ? 'Tab disabled. Click to enable' : 'Tab enabled. Click to disable'}
+        onclick=${toggleTabProperty(dispatch, 'disabled', id)}
+      />
       <${SettingsButton} icon=${disableNotifications ? 'fa-bell-slash' : 'fa-bell'}
         title=${disableNotifications ? 'Notifications disabled. Click to enable' : 'Notifications enabled. Click to disable'}
         onclick=${toggleTabProperty(dispatch, 'disableNotifications', id)}
@@ -205,7 +236,7 @@ const TabEntry = ({dispatch, id, expanded, url, disableNotifications = false, ..
         onclick=${() => dispatch({type: ACTIONS.REMOVE, payload: {id}})}
       /> 
     </div>
-    <${TabAdvancedSettings} dispatch=${dispatch} id=${id} expanded=${expanded}
+    <${TabAdvancedSettings} dispatch=${dispatch} id=${id} expanded=${expanded} disabled=${disabled}
       ...${tab}
     />
   </div>
@@ -240,7 +271,20 @@ const Settings = () => {
   });
   const cancel = () => ipcRenderer.send(APP_EVENTS.closeDialog);
   return html`
-  <h1 class="title">Settings</h1>
+  <${TopBar}
+    fixed=${true} title='Settings'
+    endComponents=${html`
+      <div class="navbar-item field is-grouped">
+        <div class="control">
+          <button class="settings__submit button is-link"
+            disabled=${!state.canSave || state.invalidTabs.size !== 0} onclick=${save}>Ok</button>
+        </div>
+        <div class="control">
+          <button class="settings__cancel button is-link is-light" onclick=${cancel}>Cancel</button>
+        </div>
+      </div>
+    `}
+  />
   <div class="container">
     <div class="form">
       <nav class="panel">
@@ -255,12 +299,12 @@ const Settings = () => {
               onkeydown=${onNewKeyDown}
             />
           </div>
-          <${SettingsButton} icon='fa-plus' onclick=${addTab} disabled=${!state.newTabValid} /> 
+          <${SettingsButton} icon='fa-plus' onclick=${addTab} disabled=${!state.newTabValid} />
         </div>
         <div class="settings__tabs container field">
           ${state.tabs.map(tab => (html`
             <${TabEntry}
-              dispatch=${dispatch} expanded=${state.expandedTabs.includes(tab.id)}
+              dispatch=${dispatch} expanded=${state.expandedTabs.includes(tab.id)} invalidTabs=${state.invalidTabs}
               ...${tab}
             />
         `))}
@@ -276,7 +320,7 @@ const Settings = () => {
       <${DictionaryEntry} dispatch=${dispatch} dictionaryKey=${key} name=${name}
         enabled=${enabledDictionaries.includes(key)}
       />
-    `))} 
+    `))}
           </div>
         </div>
       </nav>
@@ -297,17 +341,6 @@ const Settings = () => {
           ElectronIM version ${ELECTRONIM_VERSION}
         </div>
       </nav>
-      <div class="field is-grouped">
-        <div class="control">
-          <button class="settings__submit button is-link"
-            disabled=${!state.canSave} onclick=${save}>Ok</button>
-        </div>
-        <div class="control">
-          <button class="settings__cancel button is-link is-light" onclick=${cancel}>
-            Cancel
-          </button>
-        </div>
-      </div>
     </div>
   </div>
 `;
diff --git a/src/settings/preload.js b/src/settings/preload.js
index b1f0f4a..17d2249 100644
--- a/src/settings/preload.js
+++ b/src/settings/preload.js
@@ -13,18 +13,17 @@
    See the License for the specific language governing permissions and
    limitations under the License.
  */
-const preact = require('preact');
-const preactHooks = require('preact/hooks');
-const htm = require('htm');
 require('../main/preload');
+
 const {AVAILABLE_DICTIONARIES, getEnabledDictionaries} = require('../spell-check');
 const {loadSettings} = require('./');
 
 const settings = loadSettings();
 
-window.preact = preact;
-window.preactHooks = preactHooks;
-window.htm = htm;
+window.preact = require('preact');
+window.preactHooks = require('preact/hooks');
+window.html = require('htm').bind(window.preact.h);
+window.TopBar = require('../components').topBar(window.html);
 
 window.dictionaries = {
   available: AVAILABLE_DICTIONARIES,


Test patch:
diff --git a/src/settings/__tests__/browser-settings.test.js b/src/settings/__tests__/browser-settings.test.js
index 07d28b7..4d2d393 100644
--- a/src/settings/__tests__/browser-settings.test.js
+++ b/src/settings/__tests__/browser-settings.test.js
@@ -43,7 +43,8 @@ describe('Settings in Browser test suite', () => {
     window.preact = require('preact');
     window.preactHooks = require('preact/hooks');
     spyUseReducer = jest.spyOn(window.preactHooks, 'useReducer');
-    window.htm = require('htm');
+    window.html = require('htm').bind(window.preact.h);
+    window.TopBar = require('../../components').topBar(window.html);
     window.APP_EVENTS = {};
     window.ELECTRONIM_VERSION = '1.33.7';
     window.dictionaries = mockDictionaries;
@@ -211,6 +212,26 @@ describe('Settings in Browser test suite', () => {
     });
   });
   describe('Tab events', () => {
+    describe('Disable icon click', () => {
+      let $disableIcon;
+      beforeEach(() => {
+        $disableIcon = document.querySelector('.settings__tabs .icon .fa-eye');
+      });
+      test('with enabled tab, should disable', async () => {
+        // When
+        fireEvent.click($disableIcon);
+        // Then
+        await waitFor(() => expect($disableIcon.classList.contains('fa-eye')).toBe(false));
+        expect($disableIcon.classList.contains('fa-eye-slash')).toBe(true);
+      });
+      test('with disabled tab, should enable', async () => {
+        // When
+        fireEvent.click($disableIcon);
+        // Then
+        await waitFor(() => expect($disableIcon.classList.contains('fa-eye-slash')).toBe(false));
+        expect($disableIcon.classList.contains('fa-eye')).toBe(true);
+      });
+    });
     test('Notification disabled icon click, should enable notification', async () => {
       // Given
       const $notificationEnabledIcon = document.querySelector('.settings__tabs .icon .fa-bell-slash');
@@ -272,28 +293,6 @@ describe('Settings in Browser test suite', () => {
         expect($expandedTab.classList.contains('settings__tab--expanded')).toBe(false);
       });
       describe('Advanced settings', () => {
-        test('Disable checkbox click, enabled tab, should disable', async () => {
-          // Given
-          await dispatch({type: 'TOGGLE_TAB_EXPANDED', payload: '1'});
-          const $disableCheckBox = document.querySelector('.settings__tab-advanced .checkbox .fa-eye');
-          // When
-          fireEvent.click($disableCheckBox);
-          // Then
-          await waitFor(() =>
-            expect($disableCheckBox.classList.contains('fa-eye')).toBe(false));
-          expect($disableCheckBox.classList.contains('fa-eye-slash')).toBe(true);
-        });
-        test('Disable checkbox click, disabled tab, should enable', async () => {
-          // Given
-          await dispatch({type: 'TOGGLE_TAB_EXPANDED', payload: '2'});
-          const $disableCheckBox = document.querySelector('.settings__tab-advanced .checkbox .fa-eye-slash');
-          // When
-          fireEvent.click($disableCheckBox);
-          // Then
-          await waitFor(() =>
-            expect($disableCheckBox.classList.contains('fa-eye-slash')).toBe(false));
-          expect($disableCheckBox.classList.contains('fa-eye')).toBe(true);
-        });
         test('Sandbox checkbox click, sandboxed session, should unlock', async () => {
           // Given
           await dispatch({type: 'TOGGLE_TAB_EXPANDED', payload: '1'});
@@ -318,6 +317,26 @@ describe('Settings in Browser test suite', () => {
         });
       });
     });
+    describe('URL edit', () => {
+      let $input;
+      let $submitButton;
+      beforeEach(() => {
+        $input = document.querySelector('.settings__tab[data-id="1"] .input');
+        $submitButton = document.querySelector('.settings__submit');
+      });
+      test('with valid URL, should enable save button', async () => {
+        // When
+        fireEvent.input($input, {target: {value: 'https://info.cern.ch'}});
+        // Then
+        await waitFor(() => expect($submitButton.hasAttribute('disabled')).toBe(false));
+      });
+      test('with invalid URL, should disable save button', async () => {
+        // When
+        fireEvent.input($input, {target: {value: 'missing-protocol-info.cern.ch'}});
+        // Then
+        await waitFor(() => expect($submitButton.hasAttribute('disabled')).toBe(true));
+      });
+    });
   });
   describe('Dictionary events', () => {
     let $dictionaries;