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

style: enhancements for responsive design #930

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
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
215 changes: 215 additions & 0 deletions cypress/e2e/item/mobileView/mobileItemActions.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,215 @@
import { Context, FlagType, ItemTagType } from '@graasp/sdk';

import { DISPLAY_CO_EDITORS_OPTIONS } from '@/config/constants';

import { buildItemPath } from '../../../../src/config/paths';
import {
COLLAPSE_ITEM_BUTTON_CLASS,
EDIT_ITEM_BUTTON_CLASS,
FAVORITE_ITEM_BUTTON_CLASS,
HIDDEN_ITEM_BUTTON_CLASS,
HOME_MODAL_ITEM_ID,
ITEM_CHATBOX_BUTTON_ID,
ITEM_FORM_CONFIRM_BUTTON_ID,
ITEM_INFORMATION_BUTTON_ID,
ITEM_MENU_COPY_BUTTON_CLASS,
ITEM_MENU_FLAG_BUTTON_CLASS,
ITEM_MENU_RECYCLE_BUTTON_CLASS,
ITEM_MENU_SHORTCUT_BUTTON_CLASS,
ITEM_SETTINGS_BUTTON_CLASS,
ITEM_SETTINGS_CONTAINER_ID,
MOBILE_MORE_ACTIONS_BUTTON_ID,
MODE_GRID_BUTTON_ID,
PIN_ITEM_BUTTON_CLASS,
PUBLISH_ITEM_BUTTON_CLASS,
SHARE_ITEM_BUTTON_CLASS,
buildCoEditorSettingsRadioButtonId,
buildDownloadButtonId,
buildFlagListItemId,
buildShortLinkUrlTextId,
} from '../../../../src/config/selectors';
import { ITEM_LAYOUT_MODES } from '../../../../src/enums';
import { SAMPLE_ITEMS } from '../../../fixtures/items';
import { buildGraaspBuilderView } from '../../../support/paths';

const visitItem = ({ id }: { id: string }) => {
cy.visit(buildItemPath(id));
cy.switchMode(ITEM_LAYOUT_MODES.LIST);
};

const mainButtons = [
ITEM_INFORMATION_BUTTON_ID,
ITEM_CHATBOX_BUTTON_ID,
MODE_GRID_BUTTON_ID,
];

describe('check item actions within mobile view', () => {
beforeEach(() => {
// run these tests as if in a mobile
cy.viewport(400, 750);
});
it('check view, chat, and info buttons exits', () => {
const { id } = SAMPLE_ITEMS.items[1];
cy.setUpApi(SAMPLE_ITEMS);
visitItem({ id });

Comment on lines +52 to +55
Copy link
Member

Choose a reason for hiding this comment

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

These lines are repeated in each test, put them in the beforeEach.

mainButtons.forEach((btn) => {
cy.get(`#${btn}`).should('exist');
});
});

it(`check item edit`, () => {
const { id } = SAMPLE_ITEMS.items[1];
cy.setUpApi(SAMPLE_ITEMS);
visitItem({ id });

cy.get(`#${MOBILE_MORE_ACTIONS_BUTTON_ID}`).click();
cy.get(`.${EDIT_ITEM_BUTTON_CLASS}`).click();
cy.get(`#${ITEM_FORM_CONFIRM_BUTTON_ID}`).should('exist');
});

it('check download button exist', () => {
const { id } = SAMPLE_ITEMS.items[1];
cy.setUpApi(SAMPLE_ITEMS);
visitItem({ id });

cy.get(`#${MOBILE_MORE_ACTIONS_BUTTON_ID}`).click();
cy.get(`#${buildDownloadButtonId(id)}`).should('exist');
});

it('check copy button exists and open copy modal', () => {
const { id } = SAMPLE_ITEMS.items[1];
cy.setUpApi(SAMPLE_ITEMS);
visitItem({ id });

cy.get(`#${MOBILE_MORE_ACTIONS_BUTTON_ID}`).click();
cy.get(`.${ITEM_MENU_COPY_BUTTON_CLASS}`).click();
cy.get(`#${HOME_MODAL_ITEM_ID}`).should('exist');
});

it('Add Item to favorite from mobile view', () => {
const { id } = SAMPLE_ITEMS.items[1];
cy.setUpApi(SAMPLE_ITEMS);
visitItem({ id });

cy.get(`#${MOBILE_MORE_ACTIONS_BUTTON_ID}`).click();
cy.get(`.${FAVORITE_ITEM_BUTTON_CLASS}`).click();
cy.wait('@favoriteItem').then(({ request }) => {
expect(request.url).to.contain(id);
});
});
it('Share Item Should open share modal and check builder sharing url value', () => {
const { id } = SAMPLE_ITEMS.items[1];
cy.setUpApi(SAMPLE_ITEMS);
visitItem({ id });

cy.get(`#${MOBILE_MORE_ACTIONS_BUTTON_ID}`).click();
cy.get(`.${SHARE_ITEM_BUTTON_CLASS}`).click();
cy.get(`#${buildShortLinkUrlTextId(Context.Builder)}`).should(
'contain',
buildGraaspBuilderView(id),
);
});
it('Publish Button, check co-editor settings', () => {
const { id } = SAMPLE_ITEMS.items[1];
cy.setUpApi(SAMPLE_ITEMS);
visitItem({ id });

cy.get(`#${MOBILE_MORE_ACTIONS_BUTTON_ID}`).click();
cy.get(`.${PUBLISH_ITEM_BUTTON_CLASS}`).click();
Object.values(DISPLAY_CO_EDITORS_OPTIONS).forEach((option) => {
const displayTags = cy.get(
`#${buildCoEditorSettingsRadioButtonId(option.value)}`,
);
displayTags.should('be.visible');
Comment on lines +121 to +124
Copy link
Member

Choose a reason for hiding this comment

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

Do not assign to a variable. It is prefered to simply chain.

Suggested change
const displayTags = cy.get(
`#${buildCoEditorSettingsRadioButtonId(option.value)}`,
);
displayTags.should('be.visible');
cy.get(
`#${buildCoEditorSettingsRadioButtonId(option.value)}`,
).should('be.visible');

});
});
it('Shortcut button should open modal', () => {
const { id } = SAMPLE_ITEMS.items[1];
cy.setUpApi(SAMPLE_ITEMS);
visitItem({ id });

cy.get(`#${MOBILE_MORE_ACTIONS_BUTTON_ID}`).click();
cy.get(`.${ITEM_MENU_SHORTCUT_BUTTON_CLASS}`).click();
cy.get(`#${HOME_MODAL_ITEM_ID}`).should('exist');
});
it('hide item', () => {
const { id } = SAMPLE_ITEMS.items[1];
cy.setUpApi(SAMPLE_ITEMS);
visitItem({ id });

cy.get(`#${MOBILE_MORE_ACTIONS_BUTTON_ID}`).click();
cy.get(`.${HIDDEN_ITEM_BUTTON_CLASS}`).click();
cy.wait(`@postItemTag-${ItemTagType.Hidden}`).then(
({ request: { url } }) => {
expect(url).to.contain(ItemTagType.Hidden);
expect(url).to.contain(id);
},
);
});
it('collapse item', () => {
const { id } = SAMPLE_ITEMS.items[1];
cy.setUpApi(SAMPLE_ITEMS);
visitItem({ id });

cy.get(`#${MOBILE_MORE_ACTIONS_BUTTON_ID}`).click();
cy.get(`.${COLLAPSE_ITEM_BUTTON_CLASS}`).click();
cy.wait('@editItem').then(
({
response: {
body: { id: payloadId, settings },
},
}) => {
expect(payloadId).to.equal(id);
expect(settings.isCollapsible).equals(true);
},
);
});
it('Pin item', () => {
const { id } = SAMPLE_ITEMS.items[1];
cy.setUpApi(SAMPLE_ITEMS);
visitItem({ id });

cy.get(`#${MOBILE_MORE_ACTIONS_BUTTON_ID}`).click();
cy.get(`.${PIN_ITEM_BUTTON_CLASS}`).click();
cy.wait(`@editItem`).then(
({
request: {
body: { settings },
},
}) => {
expect(settings.isPinned).equals(true);
},
);
});
it.only('Recycle item', () => {
Copy link
Member

Choose a reason for hiding this comment

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

Remove only

const { id } = SAMPLE_ITEMS.items[1];
cy.setUpApi(SAMPLE_ITEMS);
visitItem({ id });
cy.get(`#${MOBILE_MORE_ACTIONS_BUTTON_ID}`).click();
cy.get(`.${ITEM_MENU_RECYCLE_BUTTON_CLASS}`).click();

cy.wait('@recycleItems').then(({ request: { url } }) => {
expect(url).to.contain(id);
});
});
it('Flag item false information option should exist', () => {
const { id } = SAMPLE_ITEMS.items[1];
const type = FlagType.FalseInformation;

cy.setUpApi(SAMPLE_ITEMS);
visitItem({ id });
cy.get(`#${MOBILE_MORE_ACTIONS_BUTTON_ID}`).click();
cy.get(`.${ITEM_MENU_FLAG_BUTTON_CLASS}`).click();
cy.get(`#${buildFlagListItemId(type)}`).should('exist');
Copy link
Member

Choose a reason for hiding this comment

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

There is a difference between should exist and should be visible:

  • exist: only checks for existence in the DOM, if it is disabled, or hidden by CSS this will pass
  • be.visible: asserts that it is visible by the user, if it is hidden by CSS this will fail.
    In general you want to assert for visibility more than existence.

});
it('Item settings', () => {
const { id } = SAMPLE_ITEMS.items[1];

cy.setUpApi(SAMPLE_ITEMS);
visitItem({ id });
cy.get(`#${MOBILE_MORE_ACTIONS_BUTTON_ID}`).click();
cy.get(`.${ITEM_SETTINGS_BUTTON_CLASS}`).click();
cy.get(`#${ITEM_SETTINGS_CONTAINER_ID}`).should('exist');
Copy link
Member

Choose a reason for hiding this comment

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

Same here

});
});
2 changes: 1 addition & 1 deletion src/components/common/BackButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const BackButton = ({ onClick }: Props): JSX.Element => {
return (
<Tooltip title={t(BUILDER.BACK)}>
<IconButton onClick={onClick}>
<ArrowCircleLeftRoundedIcon fontSize="large" />
Copy link
Member

Choose a reason for hiding this comment

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

I think @phypilia made it big on purpose. But we could disable the padding to minimise the space taken by the navigation.

<ArrowCircleLeftRoundedIcon />
</IconButton>
</Tooltip>
);
Expand Down
16 changes: 14 additions & 2 deletions src/components/common/EditButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import { useState } from 'react';
import { Dialog } from '@mui/material';

import { DiscriminatedItem, ItemType } from '@graasp/sdk';
import { EditButton as GraaspEditButton } from '@graasp/ui';
import {
ActionButton,
ActionButtonVariant,
EditButton as GraaspEditButton,
} from '@graasp/ui';

import { useBuilderTranslation } from '../../config/i18n';
import {
Expand All @@ -22,14 +26,21 @@ import NameForm from '../item/form/NameForm';

type Props = {
item: DiscriminatedItem;
type?: ActionButtonVariant;
onClick?: () => void;
};

const EditButton = ({ item }: Props): JSX.Element => {
const EditButton = ({
item,
type = ActionButton.ICON_BUTTON,
onClick,
}: Props): JSX.Element => {
const { t: translateBuilder } = useBuilderTranslation();
const [open, setOpen] = useState(false);

const handleEdit = () => {
setOpen(true);
onClick?.();
};

const typeToFormComponent = (): EditModalContentType => {
Expand Down Expand Up @@ -66,6 +77,7 @@ const EditButton = ({ item }: Props): JSX.Element => {
ariaLabel={translateBuilder(BUILDER.EDIT_ITEM_BUTTON)}
className={EDIT_ITEM_BUTTON_CLASS}
onClick={handleEdit}
type={type}
/>
</>
);
Expand Down
54 changes: 39 additions & 15 deletions src/components/common/PublishButton.tsx
Copy link
Member

Choose a reason for hiding this comment

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

This button does not exist in UI ? maybe we should create it ?

Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { Link } from 'react-router-dom';

import { ListItemIcon, MenuItem } from '@mui/material';
import IconButton from '@mui/material/IconButton';
import Tooltip from '@mui/material/Tooltip';

import { LibraryIcon } from '@graasp/ui';
import { ActionButton, ActionButtonVariant, LibraryIcon } from '@graasp/ui';

import { buildItemPublishPath } from '@/config/paths';

Expand All @@ -16,26 +17,49 @@ import { BUILDER } from '../../langs/constants';

type Props = {
itemId: string;
type?: ActionButtonVariant;
};

const PublishButton = ({ itemId }: Props): JSX.Element => {
const PublishButton = ({
itemId,
type = ActionButton.ICON_BUTTON,
}: Props): JSX.Element => {
const { t: translateBuilder } = useBuilderTranslation();

const title = translateBuilder(BUILDER.LIBRARY_SETTINGS_BUTTON_TITLE);

return (
<Tooltip title={title}>
<IconButton
aria-label={title}
className={PUBLISH_ITEM_BUTTON_CLASS}
id={buildPublishButtonId(itemId)}
to={buildItemPublishPath(itemId)}
component={Link}
>
<LibraryIcon size={24} showSetting primaryColor="#777" />
</IconButton>
</Tooltip>
);
switch (type) {
case ActionButton.MENU_ITEM:
return (
<MenuItem
key={title}
className={PUBLISH_ITEM_BUTTON_CLASS}
id={buildPublishButtonId(itemId)}
component={Link}
to={buildItemPublishPath(itemId)}
>
<ListItemIcon>
<LibraryIcon size={24} showSetting primaryColor="#777" />
Copy link
Member

Choose a reason for hiding this comment

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

You can define this outside the component to reduce duplication and ensure that it stays consistent if we refactor it.

</ListItemIcon>
{title}
</MenuItem>
);
case ActionButton.ICON_BUTTON:
default:
return (
<Tooltip title={title}>
<IconButton
aria-label={title}
className={PUBLISH_ITEM_BUTTON_CLASS}
id={buildPublishButtonId(itemId)}
to={buildItemPublishPath(itemId)}
component={Link}
>
<LibraryIcon size={24} showSetting primaryColor="#777" />
</IconButton>
</Tooltip>
);
}
};

export default PublishButton;
20 changes: 16 additions & 4 deletions src/components/common/ShareButton.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { Link } from 'react-router-dom';

import { ShareButton as GraaspShareButton } from '@graasp/ui';
import { styled } from '@mui/material';

import {
ActionButtonVariant,
ShareButton as GraaspShareButton,
} from '@graasp/ui';

import { buildItemSharePath } from '@/config/paths';

Expand All @@ -13,20 +18,27 @@ import { BUILDER } from '../../langs/constants';

type Props = {
itemId: string;
type?: ActionButtonVariant;
};

const ShareButton = ({ itemId }: Props): JSX.Element => {
const StyledLink = styled(Link)(() => ({
textDecoration: 'none',
color: 'black',
}));
Comment on lines +24 to +27
Copy link
Member

Choose a reason for hiding this comment

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

This sort of component is already defined in other places, we could put them together for re-use.


const ShareButton = ({ itemId, type }: Props): JSX.Element => {
const { t: translateBuilder } = useBuilderTranslation();

return (
<Link to={buildItemSharePath(itemId)}>
<StyledLink to={buildItemSharePath(itemId)}>
<GraaspShareButton
tooltip={translateBuilder(BUILDER.SHARE_ITEM_BUTTON)}
ariaLabel={translateBuilder(BUILDER.SHARE_ITEM_BUTTON)}
className={SHARE_ITEM_BUTTON_CLASS}
id={buildShareButtonId(itemId)}
type={type}
/>
</Link>
</StyledLink>
);
};

Expand Down
Loading
Loading