Article layout (#14753)

* style updates

* author and tools

* overflow hidden for avatars because geeeez

* font smoothing

* reverting little change

* specs

* preview

* css fix

* avatar fix'

* No longer eager load user profile when showing articles.

articles show view no longer needs user profile, as it no longer
renders the user profile preview.

* tags

* tags

* tags reverts

* typo

* dropping initializer for user cards previews on article page

* spec

* remove spec

* revert

* Update app/decorators/article_decorator.rb

Co-authored-by: Michael Kohl <me@citizen428.net>

* Update spec/requests/stories_show_spec.rb

Co-authored-by: Michael Kohl <me@citizen428.net>

* yoloing outline change on buttons because i couldn't see any visual difference

Co-authored-by: Dan Uber <dan@forem.com>
Co-authored-by: Michael Kohl <me@citizen428.net>
This commit is contained in:
ludwiczakpawel 2021-09-30 19:08:06 +02:00 committed by GitHub
parent 2a4d1a0622
commit 096cf18b10
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
16 changed files with 80 additions and 429 deletions

View file

@ -17,7 +17,7 @@ function addRelevantButtonsToArticle(user) {
if (parseInt(articleContainer.dataset.authorId, 10) === user.id) {
actions.push(
`<a class="crayons-btn crayons-btn--s crayons-btn--secondary" href="${articleContainer.dataset.path}/edit" rel="nofollow">Edit</a>`,
`<a class="crayons-btn crayons-btn--s crayons-btn--ghost px-2" href="${articleContainer.dataset.path}/edit" rel="nofollow">Edit</a>`,
);
let clickToEditButton = document.getElementById('author-click-to-edit');
@ -27,12 +27,12 @@ function addRelevantButtonsToArticle(user) {
if (published === true) {
actions.push(
`<a class="crayons-btn crayons-btn--s crayons-btn--secondary ml-1" href="${articleContainer.dataset.path}/manage" rel="nofollow">Manage</a>`,
`<a class="crayons-btn crayons-btn--s crayons-btn--ghost px-2" href="${articleContainer.dataset.path}/manage" rel="nofollow">Manage</a>`,
);
}
actions.push(
`<a class="crayons-btn crayons-btn--s crayons-btn--secondary ml-1" href="${articleContainer.dataset.path}/stats" rel="nofollow">Stats</a>`,
`<a class="crayons-btn crayons-btn--s crayons-btn--ghost px-2" href="${articleContainer.dataset.path}/stats" rel="nofollow">Stats</a>`,
);
}
@ -42,7 +42,7 @@ function addRelevantButtonsToArticle(user) {
// if there's already a pinned post different from the current one
if (user.admin) {
actions.push(
`<a class="crayons-btn crayons-btn--s crayons-btn--secondary ml-1" href="/admin/content_manager/articles/${articleId}" data-no-instant>Admin</a>`,
`<a class="crayons-btn crayons-btn--s crayons-btn--ghost px-2" href="/admin/content_manager/articles/${articleId}" data-no-instant>Admin</a>`,
);
}

View file

@ -43,6 +43,9 @@
// Content fonts
body {
-webkit-font-smoothing: antialiased;
-moz-osx-font-smoothing: grayscale;
&.serif-article-body {
--content-font-family: var(--ff-serif);
}

View file

@ -19,6 +19,8 @@
background-color: $background;
width: var(--su-6);
height: var(--su-6);
overflow: hidden;
vertical-align: middle;
&__image {
border-radius: $radius;

View file

@ -42,7 +42,6 @@
padding: calc(var(--vertical-padding) - var(--border-width))
calc(var(--horizontal-padding) - var(--border-width));
border-radius: var(--radius);
outline: none;
font-family: inherit;
font-size: var(--font-size);
line-height: var(--su-6); // intentionally not using $lh-* variable.

View file

@ -77,7 +77,10 @@
padding-bottom: 42%;
background-size: cover;
background-position: center center;
border-radius: var(--radius) var(--radius) 0 0;
@media (min-width: $breakpoint-s) {
border-radius: var(--radius) var(--radius) 0 0;
}
}
&__body {

View file

@ -48,6 +48,16 @@
}
}
&__actions {
&:empty {
display: none;
}
background: var(--accent-warning-a10);
border: 1px solid var(--accent-warning-a10);
border-radius: var(--radius);
padding: var(--su-1);
}
&__subheader {
font-size: var(--fs-base);
color: var(--base-60);

View file

@ -34,7 +34,7 @@ class StoriesController < ApplicationController
def show
@story_show = true
path = "/#{params[:username].downcase}/#{params[:slug]}"
if (@article = Article.includes(user: :profile).find_by(path: path)&.decorate)
if (@article = Article.includes(:user).find_by(path: path)&.decorate)
handle_article_show
elsif (@article = Article.find_by(slug: params[:slug])&.decorate)
handle_possible_redirect

View file

@ -99,7 +99,7 @@ class ArticleDecorator < ApplicationDecorator
def co_author_name_and_path
co_authors.map do |user|
"<b><a href=\"#{user.path}\">#{user.name}</a></b>"
%(<a href="#{user.path}" class="crayons-link">#{user.name}</a>)
end.to_sentence
end

View file

@ -16,7 +16,7 @@ function titleArea({
tags = tagArray.map((tag) => {
return (
tag.length > 0 && (
<span className="crayons-tag mr-2">
<span className="crayons-tag">
<span className="crayons-tag__prefix">#</span>
{tag}
</span>
@ -26,7 +26,7 @@ function titleArea({
}
// The v2 editor stores its cover image in articleState.mainImage, while the v1 editor
// stores it as previewRespose.cover_image. When previewing, we handle both by
// stores it as previewResponse.cover_image. When previewing, we handle both by
// defaulting to setting the cover image to the mainImage on the article (v2),
// and only using the cover image from the previewResponse if it exists (v1).
let coverImage = articleState.mainImage || '';
@ -60,11 +60,11 @@ function titleArea({
{!errors && markdownLintErrors?.length > 0 && (
<AccessibilitySuggestions markdownLintErrors={markdownLintErrors} />
)}
<h1 className="fs-4xl l:fs-5xl fw-bold s:fw-heavy lh-tight mb-6 spec-article__title">
<h1 className="fs-3xl m:fs-4xl l:fs-5xl fw-bold s:fw-heavy lh-tight mb-2 spec-article__title">
{previewTitle}
</h1>
<div className="spec-article__tags">{tags}</div>
<div className="spec-article__tags color-base-60">{tags}</div>
</div>
</header>
);

View file

@ -1,12 +1,8 @@
import { h, render } from 'preact';
import { Snackbar, addSnackbarItem } from '../Snackbar';
import { addFullScreenModeControl } from '../utilities/codeFullscreenModeSwitcher';
import { initializeDropdown } from '../utilities/dropdownUtils';
import { embedGists } from '../utilities/gist';
import {
initializeDropdown,
getDropdownRepositionListener,
} from '../utilities/dropdownUtils';
import { getInstantClick } from '../topNavigation/utilities';
/* global Runtime */
@ -123,42 +119,5 @@ getCsrfToken().then(async () => {
}
});
// Initialize the profile preview functionality
const profilePreviewTrigger = document.getElementById(
'profile-preview-trigger',
);
const dropdownContent = document.getElementById('profile-preview-content');
if (profilePreviewTrigger?.dataset.initialized !== 'true') {
initializeDropdown({
triggerElementId: 'profile-preview-trigger',
dropdownContentId: 'profile-preview-content',
onOpen: () => {
dropdownContent?.classList.add('showing');
},
onClose: () => {
dropdownContent?.classList.remove('showing');
},
});
profilePreviewTrigger.dataset.initialized = 'true';
}
const targetNode = document.querySelector('#comments');
targetNode && embedGists(targetNode);
// Preview card dropdowns reposition on scroll
const dropdownRepositionListener = getDropdownRepositionListener();
document.addEventListener('scroll', dropdownRepositionListener);
getInstantClick().then((ic) => {
ic.on('change', () => {
document.removeEventListener('scroll', dropdownRepositionListener);
});
});
window.addEventListener('beforeunload', () => {
document.removeEventListener('scroll', dropdownRepositionListener);
});

View file

@ -1,6 +1,5 @@
<% @actor = @article.organization || @article.user %>
<div class="crayons-article-sticky grid gap-4 break-word" id="article-show-primary-sticky-nav">
<div class="crayons-card crayons-card--secondary branded-7 p-4 pt-0 gap-4 grid" style="border-top-color: <%= Color::CompareHex.new([user_colors(@actor)[:bg], user_colors(@actor)[:text]]).brightness(0.88) %>;">
<%= render "shared/profile_card_content", actor: @actor, context: "sidebar" %>
</div>

View file

@ -97,15 +97,51 @@
<% end %>
<div class="crayons-article__header__meta">
<% if @organization %>
<a href="<%= @organization.path %>" class="flex items-center mb-4 fs-l fw-medium crayons-link">
<span class="crayons-logo crayons-logo--l mr-3">
<img src="<%= Images::Profile.call(@organization.profile_image_url, length: 50) %>" class="crayons-logo__image" alt="<%= @organization.name %> profile image">
</span>
<%= @organization.name %>
</a>
<% end %>
<h1 class="fs-3xl m:fs-4xl l:fs-5xl fw-bold s:fw-heavy lh-tight mb-4 <%= @article.title_length_classification %>">
<div class="flex s:items-start flex-col s:flex-row">
<div id="action-space" class="crayons-article__actions mb-4 s:mb-0 s:order-last"></div>
<div class="flex flex-1 mb-5 items-start">
<div class="relative">
<% if @organization %>
<a href="<%= @organization.path %>"><img src="<%= Images::Profile.call(@organization.profile_image_url, length: 50) %>" class="radius-default align-middle" width="40" height="40" alt="<%= @organization.name %> profile image"></a>
<a href="/<%= @user.username %>" class="absolute -right-2 -bottom-2 radius-full border border-solid border-2 border-base-inverted inline-flex">
<img class="radius-full align-middle" src="<%= Images::Profile.call(@user.profile_image_url, length: 50) %>" width="24" height="24" alt="<%= @user.name %>" />
</a>
<% else %>
<a href="/<%= @user.username %>"><img class="radius-full align-middle" src="<%= Images::Profile.call(@user.profile_image_url, length: 50) %>" width="40" height="40" alt="<%= @user.name %>" /></a>
<% end %>
</div>
<div class="pl-3 flex-1">
<a href="/<%= @user.username %>" class="crayons-link fw-bold"><%= @user.name %></a>
<% if @organization %>
<span class="color-base-60">for</span> <a href="<%= @organization.path %>" class="crayons-link"><%= @organization.name %></a>
<% end %>
<p class="fs-xs color-base-60">
<% if @article.published_timestamp.present? %>
Posted on
<%= local_date(@article.published_timestamp, show_year: @article.displayable_published_at.year != Time.current.year) %>
<% end %>
<% if @article.co_author_ids.present? %>
with <%= @article.co_author_name_and_path.html_safe %>
<% end %>
<% if should_show_updated_on?(@article) %>
&bull; Updated on <%= local_date(@article.edited_at, show_year: @article.edited_at.year != Time.current.year) %>
<% end %>
<% if should_show_crossposted_on?(@article) %>
&bull; Originally published at
<a href="<%= @article.canonical_url || @article.feed_source_url %>" style="color:#1395b8"><%= get_host_without_www(@article.canonical_url || @article.feed_source_url) %></a>
<% if @article.crossposted_at %>
on
<%= local_date(@article.originally_published_at || @article.published_at, show_year: (@article.originally_published_at || @article.published_at).year != Time.current.year) %>
<% end %>
<% end %>
</p>
</div>
</div>
</div>
<h1 class="fs-3xl m:fs-4xl l:fs-5xl fw-bold s:fw-heavy lh-tight mb-2 <%= @article.title_length_classification %>">
<% if @article.search_optimized_title_preamble.present? && !user_signed_in? %>
<span class="fs-xl color-base-70 block"><%= @article.search_optimized_title_preamble %></span>
<% end %>
@ -113,57 +149,12 @@
</h1>
<% cache("main-article-tags-#{@article.cached_tag_list}", expires_in: 30.hours) do %>
<div class="mb-4 spec__tags">
<div class="spec__tags">
<% @article.cached_tag_list_array.each do |tag| %>
<% tag_hash = tag_colors(tag) %>
<% styles = "" %>
<% if tag_hash[:background] %>
<% styles = "background-color:#{tag_hash[:background]};" %>
<% end %>
<% if tag_hash[:color] %>
<% styles = "#{styles}color:#{tag_hash[:color]};" %>
<% end %>
<a class="crayons-tag mr-1" href="/t/<%= tag %>" style="<%= styles %>"><span class="crayons-tag__prefix">#</span><%= tag %></a>
<a class="crayons-tag" href="/t/<%= tag %>"><span class="crayons-tag__prefix">#</span><%= tag %></a>
<% end %>
</div>
<% end %>
<div class="crayons-article__subheader">
<a href="/<%= @user.username %>" class="flex items-center mr-2 m:mr-1 mb-4 s:mb-0 fw-medium crayons-link">
<span class="crayons-avatar crayons-avatar--l mr-2 m:mr-1"><img class="crayons-avatar__image" src="<%= Images::Profile.call(@user.profile_image_url, length: 50) %>" alt="" /></span>
<span class="block m:hidden"><%= @user.name %></span>
</a>
<div class="profile-preview-card relative mr-2 mb-4 s:mb-0 fw-medium">
<button id="profile-preview-trigger" class="profile-preview-card__trigger crayons-btn crayons-btn--ghost px-2 hidden m:block" aria-label="<%= @user.name %> profile details"><%= @user.name %></button>
<%= render "/shared/profile_preview_card", actor: @user, id: "profile-preview-content" %>
</div>
<span class="fs-s mb-4 s:mb-0">
<% if @article.published_timestamp.present? %>
<%= local_date(@article.published_timestamp, show_year: @article.displayable_published_at.year != Time.current.year) %>
<% end %>
<% if @article.co_author_ids.present? %>
<em>with <%= @article.co_author_name_and_path.html_safe %></em>
<% end %>
<% if should_show_crossposted_on?(@article) %>
<em>
Originally published at
<a href="<%= @article.canonical_url || @article.feed_source_url %>" style="color:#1395b8"><%= get_host_without_www(@article.canonical_url || @article.feed_source_url) %></a>
<% if @article.crossposted_at %>
on
<%= local_date(@article.originally_published_at || @article.published_at, show_year: (@article.originally_published_at || @article.published_at).year != Time.current.year) %>
<% end %>
</em>
<% end %>
<% if should_show_updated_on?(@article) %>
・<em>Updated on <%= local_date(@article.edited_at, show_year: @article.edited_at.year != Time.current.year) %></em>
<% end %>
<span class="mr-4">・<%= @article.reading_time < 1 ? 1 : @article.reading_time %> min read</span>
</span>
<span id="action-space" class="mb-4 s:mb-0"></span>
</div>
</div>
</header>

View file

@ -1,314 +0,0 @@
describe('Preview user profile from article page', () => {
describe('mobile screens', () => {
beforeEach(() => {
cy.testSetup();
cy.fixture('users/articleEditorV1User.json').as('user');
cy.viewport('iphone-7');
cy.get('@user').then((user) => {
cy.loginAndVisit(user, '/').then(() => {
cy.findAllByRole('link', { name: 'Test article' })
.first()
.click({ force: true });
// Wait for page to load
cy.findByRole('button', { name: 'Share post options' });
});
});
});
it('should not show a preview profile details button', () => {
cy.findByRole('button', { name: 'Admin McAdmin profile details' })
.as('previewCardTrigger')
.should('not.exist');
// Check the user profile link is shown instead (there are also some in the user details card at the bottom)
cy.findAllByRole('link', { name: 'Admin McAdmin' }).should(
'have.length',
3,
);
});
});
describe("Preview profile on another user's article", () => {
beforeEach(() => {
cy.testSetup();
cy.fixture('users/articleEditorV1User.json').as('user');
cy.get('@user').then((user) => {
cy.loginAndVisit(user, '/admin_mcadmin/test-article-slug');
// Wait for the page to load
cy.findByRole('button', { name: 'Share post options' });
cy.get('[data-follow-clicks-initialized]');
});
});
it('should show a toggleable preview card for author byline', () => {
cy.findAllByRole('button', { name: 'Admin McAdmin profile details' })
.first()
.as('previewCardTrigger');
// Initializes as unexpanded
cy.get('@previewCardTrigger').should(
'have.attr',
'aria-expanded',
'false',
);
// Check the content opens and manages focus
cy.get('@previewCardTrigger').click();
cy.get('@previewCardTrigger').should(
'have.attr',
'aria-expanded',
'true',
);
cy.findAllByTestId('profile-preview-card')
.first()
.within(() => {
cy.findByRole('link', {
name: 'Admin McAdmin',
}).should('have.focus');
// Check all the expected user data sections are present
cy.findByText('Admin user summary');
cy.findByText('Software developer at Company');
cy.findByText('Edinburgh');
cy.findByText('University of Life');
cy.findByRole('button', { name: 'Follow user: Admin McAdmin' }).as(
'followUserButton',
);
cy.get('@followUserButton').should(
'have.attr',
'aria-pressed',
'false',
);
cy.get('@followUserButton').click();
cy.get('@followUserButton').should(
'have.attr',
'aria-pressed',
'true',
);
});
// Check we can close the preview dropdown
cy.get('@previewCardTrigger').click();
cy.findAllByTestId('profile-preview-card')
.first()
.should('not.be.visible');
cy.get('@previewCardTrigger').should(
'have.attr',
'aria-expanded',
'false',
);
});
it('should show a preview card for comment name', () => {
cy.findByTestId('comments-container').within(() => {
cy.findByRole('button', { name: 'Admin McAdmin profile details' }).as(
'previewCardTrigger',
);
cy.get('[data-initialized]');
cy.get('@previewCardTrigger').click();
cy.findByTestId('profile-preview-card').within(() => {
cy.findByRole('link', {
name: 'Admin McAdmin',
}).should('have.focus');
// Check all the expected user data sections are present
cy.findByText('Admin user summary');
cy.findByText('Software developer at Company');
cy.findByText('Edinburgh');
cy.findByText('University of Life');
cy.findByRole('button', {
name: 'Follow user: Admin McAdmin',
}).as('userFollowButton');
cy.get('@userFollowButton').click();
cy.get('@userFollowButton').should(
'have.attr',
'aria-pressed',
'true',
);
});
});
});
it('should update any other matching follow buttons when follow CTA is clicked', () => {
cy.get('[data-initialized]');
// Click the follow button in the author byline preview
cy.findAllByRole('button', { name: 'Admin McAdmin profile details' })
.first()
.as('bylinePreviewButton');
cy.get('@bylinePreviewButton').click();
cy.findAllByTestId('profile-preview-card')
.first()
.within(() => {
cy.findByRole('button', {
name: 'Follow user: Admin McAdmin',
}).as('userFollowButton');
cy.get('@userFollowButton').click();
cy.get('@userFollowButton').should(
'have.attr',
'aria-pressed',
'true',
);
});
// Close the preview card so the next preview button can be clicked
cy.get('@bylinePreviewButton').click();
// Check the follow button in the comment author preview card has updated
cy.findAllByRole('button', { name: 'Admin McAdmin profile details' })
.last()
.click();
cy.findAllByTestId('profile-preview-card')
.last()
.findByRole('button', { name: 'Follow user: Admin McAdmin' })
.should('have.attr', 'aria-pressed', 'true');
});
it('should detach listeners on preview card close', () => {
cy.findAllByRole('button', { name: 'Admin McAdmin profile details' })
.first()
.as('previewCardTrigger');
// Make sure button has initialized
cy.get('[data-initialized]').should('exist');
// Open the preview
cy.get('@previewCardTrigger').click();
cy.get('@previewCardTrigger').should(
'have.attr',
'aria-expanded',
'true',
);
// Close by pressing Escape
cy.get('body').type('{esc}');
cy.get('@previewCardTrigger').should(
'have.attr',
'aria-expanded',
'false',
);
cy.get('@previewCardTrigger').should('have.focus');
// Focus another item on the page
cy.findByRole('button', { name: 'Share post options' }).focus();
// Press Escape again and check the focus isn't reverted back to the dropdown trigger
cy.get('body').type('{esc}');
cy.get('@previewCardTrigger').should('not.have.focus');
// Click on a non-interactive element and check the focus isn't reverted back to the dropdown trigger
cy.findByRole('heading', { name: 'Test article' }).click();
cy.get('@previewCardTrigger').should('not.have.focus');
});
});
describe("Preview profile on user's own article", () => {
beforeEach(() => {
cy.testSetup();
cy.fixture('users/articleEditorV1User.json').as('user');
cy.get('@user').then((user) => {
cy.loginUser(user).then(() => {
cy.createArticle({
title: 'Test Article',
tags: ['beginner', 'ruby', 'go'],
content: `This is a test article's contents.`,
published: true,
}).then((response) => {
cy.visitAndWaitForUserSideEffects(response.body.current_state_path);
});
});
});
});
it('should show a preview card with Edit profile CTA', () => {
cy.findByRole('button', {
name: 'Article Editor v1 User profile details',
}).as('previewCardTrigger');
// Initializes as unexpanded
cy.get('@previewCardTrigger').should(
'have.attr',
'aria-expanded',
'false',
);
// Check the content opens and manages focus
cy.get('@previewCardTrigger').click();
cy.get('@previewCardTrigger').should(
'have.attr',
'aria-expanded',
'true',
);
cy.findByTestId('profile-preview-card').within(() => {
cy.findByRole('link', {
name: 'Article Editor v1 User',
}).should('have.focus');
// Check Follow option is replaced with Edit profile
cy.findByRole('button', { name: 'Edit profile' }).click();
});
// Check Edit profile directs to correct page
cy.url().should('contain', '/settings');
cy.findByRole('heading', {
name: 'Settings for @article_editor_v1_user',
});
});
});
describe('Preview author profile on an organization article', () => {
beforeEach(() => {
cy.testSetup();
cy.fixture('users/articleEditorV1User.json').as('user');
cy.get('@user').then((user) => {
cy.loginUser(user).then(() => {
cy.visit('/');
cy.findAllByRole('link', { name: 'Organization test article' })
.first()
.click({ force: true });
// Make sure we have arrived on the article page
cy.findByRole('button', { name: 'Share post options' });
});
});
});
it('Should show author details in the preview card', () => {
cy.findByRole('button', { name: 'Admin McAdmin profile details' }).as(
'profileDetailsButton',
);
// Make sure the button is initialized before interacting
cy.get('[data-initialized]');
cy.get('@profileDetailsButton').click();
cy.findByTestId('profile-preview-card').within(() => {
// Check user fields are present
cy.findByRole('link', {
name: 'Admin McAdmin',
}).should('have.focus');
cy.findByRole('button', { name: 'Follow user: Admin McAdmin' });
cy.findByText('Admin user summary');
cy.findByText('Software developer at Company');
cy.findByText('Edinburgh');
cy.findByText('University of Life');
});
});
});
});

View file

@ -69,7 +69,7 @@ describe('V2 Editor Post options', () => {
});
// Verify that the canonical URL is used
cy.findByText('Originally published at').should('exist');
cy.findByText(/Originally published at/).should('exist');
cy.findByRole('link', { name: 'exampleurl.com' });
});

View file

@ -134,7 +134,7 @@ RSpec.describe "StoriesShow", type: :request do
user2 = create(:user)
article.update(co_author_ids: [user2.id])
get article.path
expect(response.body).to include "<em>with <b><a href=\"#{user2.path}\">"
expect(response.body).to include %(with <a href="#{user2.path}" class="crayons-link">)
end
it "renders articles of long length without breaking" do

View file

@ -105,8 +105,7 @@ RSpec.describe "Using the editor", type: :system do
expect(page).to have_xpath("//div[@class='crayons-article__header__meta']//h1")
expect(page).to have_text("Hello")
expect(page).not_to have_text("</span>")
expect(page).to have_link("#what", href: "/t/what")
expect(page).to have_link("what", href: "/t/what")
end
end
end