maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Replace article's save button with bookmark (#17893)

Browse source
This commit is contained in:
Jeferson S. Brito 2022-08-19 13:03:57 -03:00 • committed by GitHub
parent 6297acbd5b
commit e0638d391c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 99 additions and 114 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
app/assets/javascripts/initializers/initializeReadingListIcons.js
View file

@ -7,7 +7,6 @@
function initializeReadingListIcons() {
setReadingListButtonsState();
addReadingListCountToHomePage();
addHoverEffectToReadingListButtons();
}
// set SAVE or SAVED articles buttons
@ -79,7 +78,6 @@ function reactToReadingListButtonClick(event) {
function renderButtonState(button, json) {
if (json.result === 'create') {
button.classList.add('selected');
addHoverEffectToReadingListButtons(button);
} else {
button.classList.remove('selected');
}
@ -126,28 +124,6 @@ function properButtonFromEvent(event) {
return properElement;
}
/*
Add the hover effect to reading list buttons.
This function makes use of mouseover/mouseevent bubbling behaviors to attach
only two event handlers to the articles container for performance reasons.
*/
function addHoverEffectToReadingListButtons() {
var articlesList = document.getElementsByClassName('articles-list');
Array.from(articlesList).forEach(function (container) {
// we use `bind` so that the event handler will have the correct text in its
// `this` local variable
container.addEventListener(
'mouseover',
readingListButtonMouseHandler.bind('Unsave'),
);
container.addEventListener(
'mouseout',
readingListButtonMouseHandler.bind('Saved'),
);
});
}
/*
Determines if the element is the target of the reading list button hover.
*/

25
app/assets/javascripts/utilities/buildArticleHTML.js
View file

@ -271,18 +271,25 @@ function buildArticleHTML(article, currentUserId = null) {
}
var saveButton = '';
var saveSVG =
'<svg width="24" height="24" xmlns="http://www.w3.org/2000/svg" role="presentation"><path d="M6.75 4.5h10.5a.75.75 0 01.75.75v14.357a.375.375 0 01-.575.318L12 16.523l-5.426 3.401A.375.375 0 016 19.607V5.25a.75.75 0 01.75-.75zM16.5 6h-9v11.574l4.5-2.82 4.5 2.82V6z" /></svg>';
var saveFilledSVG =
'<svg width="24" height="24" xmlns="http://www.w3.org/2000/svg" role="presentation"><path d="M6.75 4.5h10.5a.75.75 0 01.75.75v14.357a.375.375 0 01-.575.318L12 16.523l-5.426 3.401A.375.375 0 016 19.607V5.25a.75.75 0 01.75-.75z"/></svg>';
// "!=" instead of "!==" used to compare user_id and currentUserId because
// currentUserId is a String while user_id is an Integer
if (article.class_name === 'Article' && article.user_id != currentUserId) {
saveButton =
'<button type="button" id="article-save-button-' +
article.id +
'" class="crayons-btn crayons-btn--secondary crayons-btn--s bookmark-button" data-reactable-id="' +
article.id +
'">\
<span class="bm-initial">Save</span>\
<span class="bm-success">Saved</span>\
</button>';
saveButton = `
<button
type="button"
id="article-save-button-${article.id}"
class="c-btn c-btn--icon-alone bookmark-button"
data-reactable-id="${article.id}"
data-article-author-id="${article.user_id}"
aria-label="Save to reading list">
<span class="bm-initial">${saveSVG}</span>
<span class="bm-success">${saveFilledSVG}</span>
</button>
`;
} else if (article.class_name === 'User') {
saveButton = `
<button type="button"

2
app/assets/stylesheets/components/stories.scss
View file

@ -343,7 +343,7 @@
// Legacy
.crayons-story {
.crayons-btn {
.c-btn {
.bm-initial {
display: inline-flex;
}

40
app/javascript/articles/__tests__/__snapshots__/Article.test.jsx.snap
View file

@ -238,18 +238,32 @@ Object {
class="crayons-story__save"
>
<small
class="crayons-story__tertiary mr-2"
class="crayons-story__tertiary mr-2 fs-xs"
>
1 min read
</small>
<button
class="crayons-btn crayons-btn--s crayons-btn--secondary"
aria-label="Save to reading list"
aria-pressed="false"
class="c-btn c-btn--icon-alone"
data-initial-feed="true"
data-reactable-id="62407"
id="article-save-button-62407"
title="Save to reading list"
type="button"
>
Save
<svg
aria-hidden="true"
class="crayons-icon c-btn__icon"
focusable="false"
height="24"
width="24"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M6.75 4.5h10.5a.75.75 0 0 1 .75.75v14.357a.375.375 0 0 1-.575.318L12 16.523l-5.426 3.401A.375.375 0 0 1 6 19.607V5.25a.75.75 0 0 1 .75-.75zM16.5 6h-9v11.574l4.5-2.82 4.5 2.82V6z"
/>
</svg>
</button>
</div>
</div>
@ -493,18 +507,32 @@ Object {
class="crayons-story__save"
>
<small
class="crayons-story__tertiary mr-2"
class="crayons-story__tertiary mr-2 fs-xs"
>
1 min read
</small>
<button
class="crayons-btn crayons-btn--s crayons-btn--secondary"
aria-label="Save to reading list"
aria-pressed="false"
class="c-btn c-btn--icon-alone"
data-initial-feed="true"
data-reactable-id="62407"
id="article-save-button-62407"
title="Save to reading list"
type="button"
>
Save
<svg
aria-hidden="true"
class="crayons-icon c-btn__icon"
focusable="false"
height="24"
width="24"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M6.75 4.5h10.5a.75.75 0 0 1 .75.75v14.357a.375.375 0 0 1-.575.318L12 16.523l-5.426 3.401A.375.375 0 0 1 6 19.607V5.25a.75.75 0 0 1 .75-.75zM16.5 6h-9v11.574l4.5-2.82 4.5 2.82V6z"
/>
</svg>
</button>
</div>
</div>

2
app/javascript/articles/components/ReadingTime.jsx
View file

@ -4,7 +4,7 @@ import PropTypes from 'prop-types';
export const ReadingTime = ({ readingTime }) => {
// we have ` ... || null` for the case article.reading_time is undefined
return (
<small className="crayons-story__tertiary mr-2">
<small className="crayons-story__tertiary mr-2 fs-xs">
{`${readingTime < 1 ? 1 : readingTime} min read`}
</small>
);

39
app/javascript/articles/components/SaveButton.jsx
View file

@ -2,6 +2,9 @@ import { h } from 'preact';
import { useState } from 'preact/hooks';
import PropTypes from 'prop-types';
import { articlePropTypes } from '../../common-prop-types';
import { ButtonNew as Button } from '@crayons';
import BookmarkSVG from '@images/small-save.svg';
import BookmarkFilledSVG from '@images/small-save-filled.svg';
export const SaveButton = ({
article,
@ -9,55 +12,37 @@ export const SaveButton = ({
onClick,
saveable = true,
}) => {
const [buttonText, setButtonText] = useState(
isBookmarkedProps ? 'Saved' : 'Save',
);
const [isBookmarked, setIsBookmarked] = useState(isBookmarkedProps);
const mouseMove = (_e) => {
setButtonText(isBookmarked ? 'Unsave' : 'Save');
};
const mouseOut = (_e) => {
setButtonText(isBookmarked ? 'Saved' : 'Save');
};
const handleClick = (_e) => {
onClick(_e);
setButtonText(isBookmarked ? 'Save' : 'Saved');
setIsBookmarked((prevState) => !prevState);
};
if (article.class_name === 'Article' && saveable) {
return (
<button
type="button"
<Button
id={`article-save-button-${article.id}`}
className={`crayons-btn crayons-btn--s ${
isBookmarked ? 'crayons-btn--ghost' : 'crayons-btn--secondary'
}`}
variant="default"
title="Save to reading list"
aria-label="Save to reading list"
aria-pressed={isBookmarked}
icon={isBookmarked ? BookmarkFilledSVG : BookmarkSVG}
data-initial-feed
data-reactable-id={article.id}
onClick={handleClick}
onMouseMove={mouseMove}
onFocus={mouseMove}
onMouseout={mouseOut}
onBlur={mouseOut}
>
{buttonText}
</button>
/>
);
}
if (article.class_name === 'User') {
return (
<button
type="button"
<Button
className="crayons-btn crayons-btn--secondary fs-s"
data-info={`{"id":${article.id},"className":"User"}`}
data-follow-action-button
>
&nbsp;
</button>
</Button>
);
}

71
app/javascript/articles/components/__tests__/SaveButton.test.jsx
View file

@ -1,14 +1,14 @@
import { h } from 'preact';
import { fireEvent, render } from '@testing-library/preact';
import { render } from '@testing-library/preact';
import { axe } from 'jest-axe';
import { SaveButton } from '../SaveButton';
it('should not show bookmark button when saveable is false', () => {
const article = { class_name: 'Article', id: 1 };
const { queryByText } = render(
const { queryByRole } = render(
<SaveButton article={article} saveable={false} />,
);
const saveButton = queryByText('Saved');
const saveButton = queryByRole('button', { name: 'Save to reading list' });
expect(saveButton).toBeNull();
});
@ -23,75 +23,60 @@ it('should have no a11y violations', async () => {
it('should render button as bookmarked', () => {
const article = { class_name: 'Article', id: 1 };
const { queryByText } = render(<SaveButton article={article} isBookmarked />);
const saveButton = queryByText('Saved');
const { queryByRole } = render(<SaveButton article={article} isBookmarked />);
const saveButton = queryByRole('button', {
name: 'Save to reading list',
pressed: true,
});
expect(saveButton).not.toBeNull();
});
it('should button as not being bookmarked', () => {
const article = { class_name: 'Article', id: 1 };
const { queryByText } = render(
const { queryByRole } = render(
<SaveButton article={article} isBookmarked={false} />,
);
const saveButton = queryByText('Save');
const saveButton = queryByRole('button', {
name: 'Save to reading list',
pressed: false,
});
expect(saveButton).not.toBeNull();
});
it('should bookmark when it previously was not', async () => {
const article = { class_name: 'Article', id: 1 };
const { getByText, findByText } = render(
const { getByRole, findByRole } = render(
<SaveButton onClick={jest.fn()} article={article} isBookmarked={false} />,
);
const saveButton = getByText('Save');
const saveButton = getByRole('button', { name: 'Save to reading list' });
saveButton.click();
const savedButton = await findByText('Saved');
const savedButton = await findByRole('button', {
name: 'Save to reading list',
pressed: true,
});
expect(savedButton).not.toBeNull();
});
it('should unbookmark when it previously was bookmarked', async () => {
const article = { class_name: 'Article', id: 1 };
const { getByText, findByText } = render(
const { getByRole, findByRole } = render(
<SaveButton onClick={jest.fn()} article={article} isBookmarked />,
);
const savedButton = getByText('Saved');
const savedButton = getByRole('button', {
name: 'Save to reading list',
pressed: true,
});
savedButton.click();
const saveButton = await findByText('Save');
expect(saveButton).not.toBeNull();
});
it('should change text to unbookmark when hovering over button and it is bookmarked', async () => {
const article = { class_name: 'Article', id: 1 };
const { getByText, findByText } = render(
<SaveButton onClick={jest.fn()} article={article} isBookmarked />,
);
const savedButton = getByText('Saved');
fireEvent.mouseMove(savedButton);
const saveButton = await findByText('Unsave');
expect(saveButton).not.toBeNull();
});
it('should not change button text when hovering over button and it is not bookmarked', async () => {
const article = { class_name: 'Article', id: 1 };
const { getByText, findByText } = render(
<SaveButton onClick={jest.fn()} article={article} isBookmarked={false} />,
);
const savedButton = getByText('Save');
fireEvent.mouseMove(savedButton);
// We're checking for the same button text again because we need to find the element again
// again after the mouse has moved over the button.
const saveButton = await findByText('Save');
const saveButton = await findByRole('button', {
name: 'Save to reading list',
pressed: false,
});
expect(saveButton).not.toBeNull();
});

10
app/views/articles/_single_story.html.erb
View file

@ -150,13 +150,17 @@
<button
type="button"
id="article-save-button-<%= story.id %>"
class="crayons-btn crayons-btn--secondary crayons-btn--s w-max bookmark-button"
class="c-btn c-btn--icon-alone bookmark-button"
data-reactable-id="<%= story.id %>"
data-article-author-id="<%= story.user_id %>"
aria-label="<%= t("views.articles.save.aria_label") %>"
title="<%= t("views.articles.save.title") %>">
<span class="bm-initial"><%= t("views.articles.save.initial") %></span>
<span class="bm-success"><%= t("views.articles.save.success") %></span>
<span class="bm-initial">
<%= inline_svg_tag("small-save.svg", alt: "", aria_hidden: true) %>
</span>
<span class="bm-success">
<%= inline_svg_tag("small-save-filled.svg", alt: "", aria_hidden: true) %>
</span>
</button>
</div>
</div>