Skip to content

Commit

Permalink
[workspace]Fix workspace selector style alignment (#8592)
Browse files Browse the repository at this point in the history
* fix/workspace_selector_style_alignment, sortByRecentVisitedAndAlphabetical

Signed-off-by: Qxisylolo <[email protected]>

* align styles, typo

Signed-off-by: Qxisylolo <[email protected]>

* resolve comment

Signed-off-by: Qxisylolo <[email protected]>

* add i18n

Signed-off-by: Qxisylolo <[email protected]>

* fix i18n in test

Signed-off-by: Qxisylolo <[email protected]>

* Changeset file for PR #8592 created/updated

---------

Signed-off-by: Qxisylolo <[email protected]>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
(cherry picked from commit e09c137)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
1 parent e4133db commit 85d8d8e
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 74 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/8592.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
fix:
- Workspace selector style alignment ([#8592](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8592))
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ describe('<WorkspaceMenu />', () => {
const selectButton = screen.getByTestId('workspace-select-button');
fireEvent.click(selectButton);

expect(screen.getByText(`viewed ${moment(1234567890).fromNow()}`)).toBeInTheDocument();
expect(screen.getByText(`Viewed ${moment(1234567890).fromNow()}`)).toBeInTheDocument();
});

it('should be able to display empty state when the workspace list is empty', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,6 @@ import { WorkspaceUseCase } from '../../types';
import { validateWorkspaceColor } from '../../../common/utils';
import { getFirstUseCaseOfFeatureConfigs, getUseCaseUrl } from '../../utils';

function sortBy<T>(getkey: (item: T) => number | undefined) {
return (a: T, b: T): number => {
const aValue = getkey(a);
const bValue = getkey(b);

if (aValue === undefined) return 1;
if (bValue === undefined) return -1;

return bValue - aValue;
};
}

const searchFieldPlaceholder = i18n.translate('workspace.menu.search.placeholder', {
defaultMessage: 'Search workspace name',
});
Expand All @@ -48,7 +36,7 @@ const getValidWorkspaceColor = (color?: string) =>

interface UpdatedWorkspaceObject extends WorkspaceObject {
accessTimeStamp?: number;
accessTime?: string;
accessTimeDescription?: string;
}
interface Props {
coreStart: CoreStart;
Expand All @@ -57,6 +45,21 @@ interface Props {
isInTwoLines?: boolean;
}

const sortByRecentVisitedAndAlphabetical = (
ws1: UpdatedWorkspaceObject,
ws2: UpdatedWorkspaceObject
) => {
// First, sort by accessTimeStamp in descending order (if both have timestamps)
if (ws1?.accessTimeStamp && ws2?.accessTimeStamp) {
return ws2.accessTimeStamp - ws1.accessTimeStamp;

Check warning on line 54 in src/plugins/workspace/public/components/workspace_picker_content/workspace_picker_content.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/workspace/public/components/workspace_picker_content/workspace_picker_content.tsx#L54

Added line #L54 was not covered by tests
}
// If one has a timestamp and the other does not, prioritize the one with the timestamp
if (ws1.accessTimeStamp) return -1;
if (ws2.accessTimeStamp) return 1;
// If neither has a timestamp, sort alphabetically by name
return ws1.name.localeCompare(ws2.name);
};

export const WorkspacePickerContent = ({
coreStart,
registeredUseCases$,
Expand All @@ -72,18 +75,23 @@ export const WorkspacePickerContent = ({
const recentWorkspaces = recentWorkspaceManager.getRecentWorkspaces();
const updatedList = workspaceList.map((workspace) => {
const recentWorkspace = recentWorkspaces.find((recent) => recent.id === workspace.id);

if (recentWorkspace) {
return {
...workspace,
accessTimeStamp: recentWorkspace.timestamp,
accessTime: `viewed ${moment(recentWorkspace.timestamp).fromNow()}`,
};
}
return workspace as UpdatedWorkspaceObject;
return {
...workspace,
accessTimeStamp: recentWorkspace?.timestamp,
accessTimeDescription: recentWorkspace
? i18n.translate('workspace.picker.accessTime.description', {
defaultMessage: 'Viewed {timeLabel}',
values: {
timeLabel: moment(recentWorkspace.timestamp).fromNow(),
},
})
: i18n.translate('workspace.picker.accessTime.not.visited', {
defaultMessage: 'Not visited recently',
}),
};
});

return updatedList.sort(sortBy((workspace) => workspace.accessTimeStamp));
return updatedList.sort(sortByRecentVisitedAndAlphabetical);
}, [workspaceList]);

const queryFromList = ({ list, query }: { list: UpdatedWorkspaceObject[]; query: string }) => {
Expand Down Expand Up @@ -186,7 +194,7 @@ export const WorkspacePickerContent = ({
</EuiFlexItem>
<EuiFlexItem grow={1} style={{ position: 'absolute', right: '0px' }}>
<EuiText size="s" color="subdued">
<small> {workspace.accessTime}</small>
<small> {workspace.accessTimeDescription}</small>
</EuiText>
</EuiFlexItem>
</EuiFlexGroup>
Expand All @@ -211,7 +219,7 @@ export const WorkspacePickerContent = ({
</EuiFlexItem>
<EuiFlexItem>
<EuiText size="s" color="subdued">
<small> {workspace.accessTime}</small>
<small> {workspace.accessTimeDescription}</small>
</EuiText>
</EuiFlexItem>
</EuiFlexGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,32 @@
transition: none !important;
transform: none !important;
}

/* Trying to show the border label:
* The following style is referenced from data souce selector
* see file src/plugins/data_source_management/public/components/popover_button/popover_button.scss
*/
.workspaceSelectorPopoverButtonContainer {
position: relative;
background-color: $euiSideNavBackgroundColor;
border: 1px solid $euiColorMediumShade;
border-radius: 4px;

&::before {
content: attr(data-label);
position: absolute;
top: -0.2rem;
left: $euiSizeS;
font-size: 0.4rem;
line-height: 0.4rem;
padding: 0 $euiSizeXS;

/* update
* Delete the line-gradient and set the background color to euiSideNavBackgroundColor directly
*/
background-color: $euiSideNavBackgroundColor;
color: $euiTextColor;
z-index: 0;
text-transform: uppercase;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,8 @@ import { WorkspaceSelector } from './workspace_selector';
import { coreMock } from '../../../../../core/public/mocks';
import { CoreStart, DEFAULT_NAV_GROUPS, WorkspaceObject } from '../../../../../core/public';
import { BehaviorSubject } from 'rxjs';
import { IntlProvider } from 'react-intl';
import { recentWorkspaceManager } from '../../recent_workspace_manager';
jest.mock('@osd/i18n', () => ({
i18n: {
translate: (id: string, { defaultMessage }: { defaultMessage: string }) => defaultMessage,
},
}));
import { I18nProvider } from '@osd/i18n/react';
describe('<WorkspaceSelector />', () => {
let coreStartMock: CoreStart;
const navigateToApp = jest.fn();
Expand Down Expand Up @@ -52,9 +47,9 @@ describe('<WorkspaceSelector />', () => {

const WorkspaceSelectorCreatorComponent = () => {
return (
<IntlProvider locale="en">
<I18nProvider>
<WorkspaceSelector coreStart={coreStartMock} registeredUseCases$={registeredUseCases$} />
</IntlProvider>
</I18nProvider>
);
};

Expand All @@ -68,6 +63,7 @@ describe('<WorkspaceSelector />', () => {
expect(screen.getByTestId('workspace-selector-current-title')).toBeInTheDocument();
expect(screen.getByTestId('workspace-selector-current-name')).toBeInTheDocument();
});

it('should display a list of workspaces in the dropdown', () => {
jest
.spyOn(recentWorkspaceManager, 'getRecentWorkspaces')
Expand All @@ -82,11 +78,11 @@ describe('<WorkspaceSelector />', () => {
const selectButton = screen.getByTestId('workspace-selector-button');
fireEvent.click(selectButton);

expect(screen.getByTestId('workspace-menu-item-workspace-1')).toBeInTheDocument();
expect(screen.getByTestId('workspace-menu-item-workspace-2')).toBeInTheDocument();
expect(screen.getByText('workspace 1')).toBeInTheDocument();
expect(screen.getByText('workspace 2')).toBeInTheDocument();
});

it('should display viewed xx ago for recent workspaces', () => {
it('should display viewed xx ago for recent workspaces, and Not visited recently for never-visited workspace', () => {
jest
.spyOn(recentWorkspaceManager, 'getRecentWorkspaces')
.mockReturnValue([{ id: 'workspace-1', timestamp: 1234567890 }]);
Expand All @@ -95,13 +91,12 @@ describe('<WorkspaceSelector />', () => {
{ id: 'workspace-1', name: 'workspace 1', features: [] },
{ id: 'workspace-2', name: 'workspace 2', features: [] },
]);

render(<WorkspaceSelectorCreatorComponent />);

const selectButton = screen.getByTestId('workspace-selector-button');
fireEvent.click(selectButton);

expect(screen.getByText(`viewed ${moment(1234567890).fromNow()}`)).toBeInTheDocument();
expect(screen.getByText(`Viewed ${moment(1234567890).fromNow()}`)).toBeInTheDocument();
expect(screen.getByText('Not visited recently')).toBeInTheDocument();
});

it('should be able to display empty state when the workspace list is empty', () => {
Expand All @@ -127,8 +122,8 @@ describe('<WorkspaceSelector />', () => {

const searchInput = screen.getByRole('searchbox');
fireEvent.change(searchInput, { target: { value: 'works' } });
expect(screen.getByTestId('workspace-menu-item-workspace-1')).toBeInTheDocument();
expect(screen.queryByText('workspace-menu-item-workspace-1')).not.toBeInTheDocument();
expect(screen.getByText('workspace 1')).toBeInTheDocument();
expect(screen.queryByText('test 2')).not.toBeInTheDocument();
});

it('should be able to display empty state when seach is not found', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,51 +64,36 @@ export const WorkspaceSelector = ({ coreStart, registeredUseCases$ }: Props) =>
const closePopover = () => {
setPopover(false);
};

const button = currentWorkspace ? (
<EuiPanel
className="workspaceSelectorPopoverButton"
paddingSize="none"
color="transparent"
hasBorder={false}
hasShadow={false}
data-test-subj="workspace-selector-button"
onClick={onButtonClick}
>
<EuiText
size="xs"
// TODO: Use standard OuiComponent to achieve the label looks
style={{
position: 'relative',
bottom: '-10px',
zIndex: 1,
padding: '0 5px',
}}
<div className="workspaceSelectorPopoverButtonContainer" data-label="Workspace">
<EuiPanel
className="workspaceSelectorPopoverButton"
data-test-subj="workspace-selector-button"
paddingSize="s"
color="transparent"
hasBorder={false}
hasShadow={false}
onClick={onButtonClick}
>
<small className="workspaceNameLabel">
{i18n.translate('workspace.left.nav.selector.label', {
defaultMessage: 'WORKSPACE',
})}
</small>
</EuiText>
<EuiPanel paddingSize="s" borderRadius="m" color="transparent" hasBorder>
<EuiFlexGroup gutterSize="none" justifyContent="spaceBetween">
<EuiFlexGroup gutterSize="none" justifyContent="spaceBetween" responsive={false}>
<EuiFlexItem>
<EuiFlexGroup gutterSize="s" justifyContent="flexStart">
<EuiFlexGroup gutterSize="s" justifyContent="flexStart" responsive={false}>
<EuiFlexItem grow={false}>
<EuiIcon
size="l"
type={getUseCase(currentWorkspace)?.icon || 'wsSelector'}
color={getValidWorkspaceColor(currentWorkspace.color)}
/>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiFlexGroup direction="column" gutterSize="none">
<EuiFlexItem grow={false} style={{ maxWidth: '130px' }}>
<EuiFlexItem>
<EuiFlexGroup direction="column" gutterSize="none" responsive={false}>
<EuiFlexItem style={{ maxWidth: '130px' }}>
<EuiText size="s" data-test-subj="workspace-selector-current-name">
<h4 className="eui-textTruncate">{currentWorkspace.name}</h4>
</EuiText>
</EuiFlexItem>
<EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiText
size="xs"
color="subdued"
Expand All @@ -126,7 +111,7 @@ export const WorkspaceSelector = ({ coreStart, registeredUseCases$ }: Props) =>
</EuiFlexItem>
</EuiFlexGroup>
</EuiPanel>
</EuiPanel>
</div>
) : (
<EuiButton onClick={onButtonClick}>Select a Workspace</EuiButton>
);
Expand Down

0 comments on commit 85d8d8e

Please sign in to comment.