Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] UI update to notebooks and applications #2110

Merged
merged 2 commits into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions common/utils/set_nav_bread_crumbs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,19 @@ import { coreRefs } from '../../public/framework/core_refs';

export const setNavBreadCrumbs = (
parentBreadCrumb: EuiBreadcrumb[],
pageBreadCrumb: EuiBreadcrumb[]
pageBreadCrumb: EuiBreadcrumb[],
counter?: number
) => {
const isNavGroupEnabled = coreRefs?.chrome?.navGroup.getNavGroupEnabled();

const updatedPageBreadCrumb = pageBreadCrumb.map((crumb) => ({
...crumb,
text: isNavGroupEnabled && counter !== undefined ? `${crumb.text} (${counter})` : crumb.text,
}));

if (isNavGroupEnabled) {
coreRefs?.chrome?.setBreadcrumbs([...pageBreadCrumb]);
coreRefs?.chrome?.setBreadcrumbs([...updatedPageBreadCrumb]);
} else {
coreRefs?.chrome?.setBreadcrumbs([...parentBreadCrumb, ...pageBreadCrumb]);
coreRefs?.chrome?.setBreadcrumbs([...parentBreadCrumb, ...updatedPageBreadCrumb]);
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,12 @@ export function AppTable(props: AppTableProps) {
text: 'Applications',
href: '#/',
},
]
],
applications.length
);
clear();
fetchApplications();
}, []);
}, [applications.length]);

const clear = () => {
setFilters([]);
Expand Down Expand Up @@ -256,7 +257,6 @@ export function AppTable(props: AppTableProps) {
{createButtonText}
</EuiSmallButton>,
]}
badgeContent={applications.length}
/>
</EuiFlexItem>
</EuiFlexGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@ exports[`<NoteTable /> spec renders the component 1`] = `
<div
class="euiFlexGroup euiFlexGroup--gutterSmall euiFlexGroup--alignItemsCenter euiFlexGroup--directionRow euiFlexGroup--responsive"
>
<div
class="euiFlexItem euiFlexItem--flexGrowZero"
/>
<div
class="euiFlexItem"
>
Expand Down Expand Up @@ -186,47 +189,6 @@ exports[`<NoteTable /> spec renders the component 1`] = `
</div>
</div>
</div>
<div
class="euiFlexItem euiFlexItem--flexGrowZero"
>
<div
class="euiPopover euiPopover--anchorDownCenter"
>
<div
class="euiPopover__anchor"
>
<button
class="euiButton euiButton--primary euiButton--small"
data-test-subj="notebookTableActionBtn"
type="button"
>
<span
class="euiButtonContent euiButtonContent--iconRight euiButton__content"
>
<svg
aria-hidden="true"
class="euiIcon euiIcon--medium euiIcon--inherit euiIcon-isLoading euiButtonContent__icon"
focusable="false"
height="16"
role="img"
viewBox="0 0 16 16"
width="16"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M5.277 10.088c.02.014.04.03.057.047.582.55 1.134.812 1.666.812.586 0 1.84-.293 3.713-.88L9 6.212V2H7v4.212l-1.723 3.876Zm-.438.987L3.539 14h8.922l-1.32-2.969C9.096 11.677 7.733 12 7 12c-.74 0-1.463-.315-2.161-.925ZM6 2H5V1h6v1h-1v4l3.375 7.594A1 1 0 0 1 12.461 15H3.54a1 1 0 0 1-.914-1.406L6 6V2Z"
/>
</svg>
<span
class="euiButton__text"
>
Actions
</span>
</span>
</button>
</div>
</div>
</div>
</div>
<hr
class="euiHorizontalRule euiHorizontalRule--full euiHorizontalRule--marginMedium"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,14 @@ describe('<NoteTable /> spec', () => {
dateCreated: '2023-01-01 12:00:00',
dateModified: '2023-01-02 12:00:00',
}));
const utils = renderNoteTable({ notebooks });
const { getByTestId, getAllByText, ...utils } = renderNoteTable({ notebooks });
expect(utils.container.firstChild).toMatchSnapshot();

fireEvent.click(utils.getByText('Add sample notebooks'));
fireEvent.click(utils.getAllByLabelText('Select this row')[0]);
fireEvent.click(utils.getByText('Actions'));
fireEvent.click(utils.getByText('Delete'));
fireEvent.click(getByTestId('deleteSelectedNotebooks'));
expect(getAllByText('Delete 1 notebook')).toHaveLength(2);
fireEvent.click(utils.getByText('Cancel'));
fireEvent.click(utils.getAllByLabelText('Select this row')[0]);
fireEvent.click(utils.getByText('Actions'));
});

it('create notebook modal', async () => {
Expand Down Expand Up @@ -130,17 +128,16 @@ describe('<NoteTable /> spec', () => {
dateModified: 'date-modified',
},
];
const { getByText, getByLabelText, getAllByText, getByTestId } = renderNoteTable({ notebooks });
const { getByLabelText, getAllByText, getByTestId } = renderNoteTable({ notebooks });

// Select a notebook
fireEvent.click(getByLabelText('Select this row'));

// Open Actions dropdown and click Delete
fireEvent.click(getByText('Actions'));
fireEvent.click(getByText('Delete'));
// Click the delete button
fireEvent.click(getByTestId('deleteSelectedNotebooks'));

// Ensure the modal is open (you may need to adjust based on your modal implementation)
expect(getAllByText('Delete 1 notebook')).toHaveLength(1);
expect(getAllByText('Delete 1 notebook')).toHaveLength(2);

// Mock user confirmation and submit
fireEvent.input(getByTestId('delete-notebook-modal-input'), {
Expand Down Expand Up @@ -178,22 +175,21 @@ describe('<NoteTable /> spec', () => {
dateModified: 'date-modified',
},
];
const { getByText, getByLabelText, queryByText } = renderNoteTable({ notebooks });
const { getByText, getByLabelText, getAllByText, getByTestId } = renderNoteTable({ notebooks });

// Select a notebook
fireEvent.click(getByLabelText('Select this row'));

// Open Actions dropdown and click Delete
fireEvent.click(getByText('Actions'));
fireEvent.click(getByText('Delete'));
// Click the delete button
fireEvent.click(getByTestId('deleteSelectedNotebooks'));

// Ensure the modal is open
expect(getByText('Delete 1 notebook')).toBeInTheDocument();
expect(getAllByText('Delete 1 notebook')).toHaveLength(2);

// Close the delete modal
fireEvent.click(getByText('Cancel'));

// Ensure the delete modal is closed
expect(queryByText('Delete 1 notebook')).toBeNull();
expect(getAllByText('Delete 1 notebook')).toHaveLength(1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are the comments still accurate? it says ensure modal is closed, but still expects model text to be found?

Copy link
Collaborator Author

@TackAdam TackAdam Sep 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new functionality adds the delete button to the left of the search bar containing the text. Once the modal is closed the expected behavior is that it is still selected so we still have the (new) button. If the modal is open the text should appear twice.
Screenshot 2024-09-03 at 5 10 51 PM

});
});
60 changes: 17 additions & 43 deletions public/components/notebooks/components/note_table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@

import {
EuiSmallButton,
EuiContextMenuItem,
EuiContextMenuPanel,
EuiCompressedFieldSearch,
EuiFlexGroup,
EuiFlexItem,
Expand All @@ -21,15 +19,14 @@
EuiPageContentHeaderSection,
EuiPageHeader,
EuiPageHeaderSection,
EuiPopover,
EuiSpacer,
EuiTableFieldDataColumnType,
EuiText,
EuiTitle,
} from '@elastic/eui';
import truncate from 'lodash/truncate';
import moment from 'moment';
import React, { ReactElement, useEffect, useState } from 'react';
import React, { useEffect, useState } from 'react';
import { useHistory, useLocation } from 'react-router-dom';
import { ChromeBreadcrumb } from '../../../../../../src/core/public';
import {
Expand Down Expand Up @@ -75,7 +72,6 @@
}: NoteTableProps) {
const [isModalVisible, setIsModalVisible] = useState(false); // Modal Toggle
const [modalLayout, setModalLayout] = useState(<EuiOverlayMask />); // Modal Layout
const [isActionsPopoverOpen, setIsActionsPopoverOpen] = useState(false);
const [selectedNotebooks, setSelectedNotebooks] = useState<NotebookType[]>([]);
const [searchQuery, setSearchQuery] = useState('');
const location = useLocation();
Expand All @@ -89,17 +85,18 @@
text: 'Notebooks',
href: '#/',
},
]
],
notebooks.length
);
fetchNotebooks();
}, [setBreadcrumbs, parentBreadcrumb, fetchNotebooks]);
}, [setBreadcrumbs, parentBreadcrumb, fetchNotebooks, notebooks.length]);

useEffect(() => {
const url = window.location.hash.split('/');
if (url[url.length - 1] === 'create') {
createNote();
}
}, [location]);

Check warning on line 99 in public/components/notebooks/components/note_table.tsx

View workflow job for this annotation

GitHub Actions / Lint

React Hook useEffect has a missing dependency: 'createNote'. Either include it or remove the dependency array

const closeModal = () => {
setIsModalVisible(false);
Expand Down Expand Up @@ -166,31 +163,6 @@
showModal();
};

const popoverButton = (
<EuiSmallButton
data-test-subj="notebookTableActionBtn"
iconType="arrowDown"
iconSide="right"
onClick={() => setIsActionsPopoverOpen(!isActionsPopoverOpen)}
>
Actions
</EuiSmallButton>
);

const popoverItems: ReactElement[] = [
<EuiContextMenuItem
key="delete"
disabled={notebooks.length === 0 || selectedNotebooks.length === 0}
onClick={() => {
setIsActionsPopoverOpen(false);
deleteNote();
}}
data-test-subj="deleteNotebookBtn"
>
Delete
</EuiContextMenuItem>,
];

const tableColumns = [
{
field: 'path',
Expand Down Expand Up @@ -238,7 +210,6 @@
<EuiPageContent id="notebookArea">
{newNavigation ? (
<HeaderControlledComponentsWrapper
badgeContent={notebooks.length > 0 ? notebooks.length : '0'}
description={
<>
Use Notebooks to interactively and collaboratively develop rich reports backed
Expand Down Expand Up @@ -323,6 +294,19 @@
{notebooks.length > 0 ? (
<>
<EuiFlexGroup gutterSize="s" alignItems="center">
<EuiFlexItem grow={false}>
{selectedNotebooks.length > 0 && (
<EuiSmallButton
color="danger"
iconType="trash"
onClick={deleteNote}
data-test-subj="deleteSelectedNotebooks"
>
Delete {selectedNotebooks.length} notebook
{selectedNotebooks.length > 1 ? 's' : ''}
</EuiSmallButton>
)}
</EuiFlexItem>
<EuiFlexItem>
<EuiCompressedFieldSearch
fullWidth
Expand All @@ -331,16 +315,6 @@
onChange={(e) => setSearchQuery(e.target.value)}
/>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiPopover
panelPaddingSize="none"
button={popoverButton}
isOpen={isActionsPopoverOpen}
closePopover={() => setIsActionsPopoverOpen(false)}
>
<EuiContextMenuPanel items={popoverItems} size="s" />
</EuiPopover>
</EuiFlexItem>
</EuiFlexGroup>
<EuiHorizontalRule margin="m" />
<EuiInMemoryTable
Expand Down
Loading