Skip to content

Commit

Permalink
[Notebook] Browse Bar holding onto stale model, reverts changes (#7944)
Browse files Browse the repository at this point in the history
* moving rename methods to appActions

* importing back into original test

* reverting

* add the ability to change the name in the browse bar

* add test to verify entries are not being lost

* addding aria labels for tests

* when an object is changed, store the whole new object, not just the name

* typo!
  • Loading branch information
jvigliotta authored Dec 6, 2024
1 parent 14b947c commit 5bb6a18
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 11 deletions.
16 changes: 16 additions & 0 deletions e2e/appActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,21 @@ async function linkParameterToObject(page, parameterName, objectName) {
await page.getByLabel('Save').click();
}

/**
* Rename the currently viewed `domainObject` from the browse bar.
*
* @param {import('@playwright/test').Page} page
* @param {string} newName
*/
async function renameCurrentObjectFromBrowseBar(page, newName) {
const nameInput = page.getByLabel('Browse bar object name');
await nameInput.click();
await nameInput.fill('');
await nameInput.fill(newName);
// Click the browse bar container to save changes
await page.getByLabel('Browse bar', { exact: true }).click();
}

export {
createDomainObjectWithDefaults,
createExampleTelemetryObject,
Expand All @@ -693,6 +708,7 @@ export {
linkParameterToObject,
navigateToObjectWithFixedTimeBounds,
navigateToObjectWithRealTime,
renameCurrentObjectFromBrowseBar,
setEndOffset,
setFixedIndependentTimeConductorBounds,
setFixedTimeMode,
Expand Down
62 changes: 61 additions & 1 deletion e2e/tests/functional/plugins/notebook/notebook.e2e.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ This test suite is dedicated to tests which verify the basic operations surround

import { fileURLToPath } from 'url';

import { createDomainObjectWithDefaults } from '../../../../appActions.js';
import {
createDomainObjectWithDefaults,
renameCurrentObjectFromBrowseBar
} from '../../../../appActions.js';
import { copy, paste, selectAll } from '../../../../helper/hotkeys/hotkeys.js';
import * as nbUtils from '../../../../helper/notebookUtils.js';
import { expect, streamToString, test } from '../../../../pluginFixtures.js';
Expand Down Expand Up @@ -596,4 +599,61 @@ test.describe('Notebook entry tests', () => {
await expect(await page.locator(`text="${TEST_TEXT.repeat(1)}"`).count()).toEqual(1);
await expect(await page.locator(`text="${TEST_TEXT.repeat(2)}"`).count()).toEqual(0);
});

test('When changing the name of a notebook in the browse bar, new notebook changes are not lost', async ({
page
}) => {
const TEST_TEXT = 'Do not lose me!';
const FIRST_NEW_NAME = 'New Name';
const SECOND_NEW_NAME = 'Second New Name';

await page.goto(notebookObject.url);

await page.getByLabel('Expand My Items folder').click();

await renameCurrentObjectFromBrowseBar(page, FIRST_NEW_NAME);

// verify the name change in tree and browse bar
await verifyNameChange(page, FIRST_NEW_NAME);

// enter one entry
await enterAndCommitTextEntry(page, TEST_TEXT);

// verify the entry is present
await expect(await page.locator(`text="${TEST_TEXT}"`).count()).toEqual(1);

// change the name
await renameCurrentObjectFromBrowseBar(page, SECOND_NEW_NAME);

// verify the name change in tree and browse bar
await verifyNameChange(page, SECOND_NEW_NAME);

// verify the entry is still present
await expect(await page.locator(`text="${TEST_TEXT}"`).count()).toEqual(1);
});
});

/**
* Enter text into the last notebook entry and commit it.
*
* @param {import('@playwright/test').Page} page
* @param {string} text
*/
async function enterAndCommitTextEntry(page, text) {
await nbUtils.addNotebookEntry(page);
await nbUtils.enterTextInLastEntry(page, text);
await nbUtils.commitEntry(page);
}

/**
* Verify the name change in the tree and browse bar.
*
* @param {import('@playwright/test').Page} page
* @param {string} newName
*/
async function verifyNameChange(page, newName) {
await expect(
page.getByRole('treeitem').locator('.is-navigated-object .c-tree__item__name')
).toHaveText(newName);
await expect(page.getByLabel('Browse bar object name')).toHaveText(newName);
}
3 changes: 2 additions & 1 deletion src/ui/layout/BrowseBar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
at runtime from the About dialog for additional information.
-->
<template>
<div class="l-browse-bar">
<div class="l-browse-bar" aria-label="Browse bar">
<div class="l-browse-bar__start">
<button
v-if="hasParent"
Expand All @@ -35,6 +35,7 @@
</div>
<span
ref="objectName"
aria-label="Browse bar object name"
class="l-browse-bar__object-name c-object-label__name"
:class="{ 'c-input-inline': isPersistable }"
:contenteditable="isNameEditable"
Expand Down
16 changes: 7 additions & 9 deletions src/ui/router/Browse.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,11 @@ class Browse {
this.#openmct.layout.$refs.browseBar.viewKey = viewProvider.key;
}

#updateDocumentTitleOnNameMutation(newName) {
if (typeof newName === 'string' && newName !== document.title) {
document.title = newName;
this.#openmct.layout.$refs.browseBar.domainObject = {
...this.#openmct.layout.$refs.browseBar.domainObject,
name: newName
};
#handleBrowseObjectUpdate(newObject) {
this.#openmct.layout.$refs.browseBar.domainObject = newObject;

if (typeof newObject.name === 'string' && newObject.name !== document.title) {
document.title = newObject.name;
}
}

Expand Down Expand Up @@ -120,8 +118,8 @@ class Browse {
document.title = this.#browseObject.name; //change document title to current object in main view
this.#unobserve = this.#openmct.objects.observe(
this.#browseObject,
'name',
this.#updateDocumentTitleOnNameMutation.bind(this)
'*',
this.#handleBrowseObjectUpdate.bind(this)
);
const currentProvider = this.#openmct.objectViews.getByProviderKey(currentViewKey);
if (currentProvider && currentProvider.canView(this.#browseObject, this.#openmct.router.path)) {
Expand Down

0 comments on commit 5bb6a18

Please sign in to comment.