5413 - [Notebook] Various visual issues with renaming sections/pages (#5475)

* 5413 - [Notebook] Various visual issues with renaming sections/pages

* 5413 - [Notebook] Various visual issues with renaming sections/pages - 3rd Expectation

* 5413 - [Notebook] Various visual issues with renaming sections/pages - requested changes

* 5413 - [Notebook] Various visual issues with renaming sections/pages - remove a magic number (ENTER)

* 5413 - [Notebook] Various visual issues with renaming sections/pages - key codes

* 5413 - [Notebook] Various visual issues with renaming sections/pages - Separate selectability and editability actions for `Section` and `Page` components

* Fix layout of Notebook nav item to align affordance arrow

* Add e2e test stubs

Co-authored-by: Andrew Henry <akhenry@gmail.com>
Co-authored-by: Charles Hacskaylo <charlesh88@gmail.com>
Co-authored-by: Jesse Mazzella <ozyx@users.noreply.github.com>
Co-authored-by: Jesse Mazzella <jesse.d.mazzella@nasa.gov>

e2e/tests/functional/plugins/notebook/notebook.e2e.spec.js

@@ -86,6 +86,23 @@ test.describe('Notebook section tests', () => {
         //Delete 3rd section
         //1st is selected and there is no default notebook
     });
+    test.fixme('Section rename operations', async ({ page }) => {
+        // Create a new notebook
+        // Add a section
+        // Rename the section but do not confirm
+        // Keyboard press 'Escape'
+        // Verify that the section name reverts to the default name
+        // Rename the section but do not confirm
+        // Keyboard press 'Enter'
+        // Verify that the section name is updated
+        // Rename the section to "" (empty string)
+        // Keyboard press 'Enter' to confirm
+        // Verify that the section name reverts to the default name
+        // Rename the section to something long that overflows the text box
+        // Verify that the section name is not truncated while input is active
+        // Confirm the section name edit
+        // Verify that the section name is truncated now that input is not active
+    });
 });
 
 test.describe('Notebook page tests', () => {
@@ -107,6 +124,23 @@ test.describe('Notebook page tests', () => {
         //Delete 3rd page
         //First is now selected and there is no default notebook
     });
+    test.fixme('Page rename operations', async ({ page }) => {
+        // Create a new notebook
+        // Add a page
+        // Rename the page but do not confirm
+        // Keyboard press 'Escape'
+        // Verify that the page name reverts to the default name
+        // Rename the page but do not confirm
+        // Keyboard press 'Enter'
+        // Verify that the page name is updated
+        // Rename the page to "" (empty string)
+        // Keyboard press 'Enter' to confirm
+        // Verify that the page name reverts to the default name
+        // Rename the page to something long that overflows the text box
+        // Verify that the page name is not truncated while input is active
+        // Confirm the page name edit
+        // Verify that the page name is truncated now that input is not active
+    });
 });
 
 test.describe('Notebook search tests', () => {

src/plugins/notebook/components/PageComponent.vue

@@ -12,8 +12,10 @@
     <template v-if="!page.isLocked">
         <div
             class="c-list__item__name js-list__item__name"
+            :class="[{ 'c-input-inline': isSelected }]"
             :data-id="page.id"
-            :contenteditable="true"
+            :contenteditable="isSelected"
+            @keydown.escape="updateName"
             @keydown.enter="updateName"
             @blur="updateName"
         >{{ pageName }}</div>
@@ -32,8 +34,9 @@
 </template>
 
 <script>
-import PopupMenu from './PopupMenu.vue';
+import { KEY_ENTER, KEY_ESCAPE } from '../utils/notebook-key-code';
 import RemoveDialog from '../utils/removeDialog';
+import PopupMenu from './PopupMenu.vue';
 
 export default {
     components: {
@@ -107,36 +110,39 @@ export default {
             removeDialog.show();
         },
         selectPage(event) {
-            const target = event.target;
+            const { target: { dataset: { id } } } = event;
-            const id = target.dataset.id;
 
-            if (!this.page.isLocked) {
+            if (this.isSelected || !id) {
-                const page = target.closest('.js-list__item');
-                const input = page.querySelector('.js-list__item__name');
-
-                if (page.className.indexOf('is-selected') > -1) {
-                    input.classList.add('c-input-inline');
-
-                    return;
-                }
-            }
-
-            if (!id) {
                 return;
             }
 
             this.$emit('selectPage', id);
         },
-        updateName(event) {
+        renamePage(target) {
-            const target = event.target;
+            if (!target) {
-            const name = target.textContent.toString();
-            target.classList.remove('c-input-inline');
-
-            if (name === '' || this.page.name === name) {
                 return;
             }
 
-            this.$emit('renamePage', Object.assign(this.page, { name }));
+            target.textContent = target.textContent ? target.textContent.trim() : `Unnamed ${this.pageTitle}`;
+
+            if (this.page.name === target.textContent) {
+                return;
+            }
+
+            this.$emit('renamePage', Object.assign(this.page, { name: target.textContent }));
+        },
+        updateName(event) {
+            const { target, keyCode, type } = event;
+
+            if (keyCode === KEY_ESCAPE) {
+                target.textContent = this.page.name;
+            } else if (keyCode === KEY_ENTER || type === 'blur') {
+                this.renamePage(target);
+            }
+
+            target.scrollLeft = '0';
+
+            target.blur();
         }
     }
 };

src/plugins/notebook/components/SectionComponent.vue

@@ -7,8 +7,10 @@
 >
     <span
         class="c-list__item__name js-list__item__name"
+        :class="[{ 'c-input-inline': isSelected && !section.isLocked }]"
         :data-id="section.id"
-        contenteditable="true"
+        :contenteditable="isSelected && !section.isLocked"
+        @keydown.escape="updateName"
         @keydown.enter="updateName"
         @blur="updateName"
     >{{ sectionName }}</span>
@@ -20,8 +22,9 @@
 </template>
 
 <script>
-import PopupMenu from './PopupMenu.vue';
+import { KEY_ENTER, KEY_ESCAPE } from '../utils/notebook-key-code';
 import RemoveDialog from '../utils/removeDialog';
+import PopupMenu from './PopupMenu.vue';
 
 export default {
     components: {
@@ -96,36 +99,39 @@ export default {
             removeDialog.show();
         },
         selectSection(event) {
-            const target = event.target;
+            const { target: { dataset: { id } } } = event;
-            const id = target.dataset.id;
 
-            if (!this.section.isLocked) {
+            if (this.isSelected || !id) {
-                const section = target.closest('.js-list__item');
-                const input = section.querySelector('.js-list__item__name');
-
-                if (section.className.indexOf('is-selected') > -1) {
-                    input.classList.add('c-input-inline');
-
-                    return;
-                }
-            }
-
-            if (!id) {
                 return;
             }
 
             this.$emit('selectSection', id);
         },
-        updateName(event) {
+        renameSection(target) {
-            const target = event.target;
+            if (!target) {
-            target.classList.remove('c-input-inline');
-            const name = target.textContent.trim();
-
-            if (name === '' || this.section.name === name) {
                 return;
             }
 
-            this.$emit('renameSection', Object.assign(this.section, { name }));
+            target.textContent = target.textContent ? target.textContent.trim() : `Unnamed ${this.sectionTitle}`;
+
+            if (this.section.name === target.textContent) {
+                return;
+            }
+
+            this.$emit('renameSection', Object.assign(this.section, { name: target.textContent }));
+        },
+        updateName(event) {
+            const { target, keyCode, type } = event;
+
+            if (keyCode === KEY_ESCAPE) {
+                target.textContent = this.section.name;
+            } else if (keyCode === KEY_ENTER || type === 'blur') {
+                this.renameSection(target);
+            }
+
+            target.scrollLeft = '0';
+
+            target.blur();
         }
     }
 };

src/plugins/notebook/components/sidebar.scss

@@ -88,7 +88,7 @@
         }
 
         &__name {
-            flex: 0 1 auto;
+            flex: 1 1 auto;
         }
 
         &__menu-indicator {

src/plugins/notebook/utils/notebook-key-code.js

@@ -0,0 +1,3 @@
+// Key codes for `KeyboardEvent.keyCode`.
+export const KEY_ENTER = 13;
+export const KEY_ESCAPE = 27;

src/styles/notebook.scss

@@ -744,3 +744,9 @@ body.mobile {
         }
     }
 }
+
+.c-list__item {
+    &__name:focus {
+        text-overflow: clip;
+    }
+}