Read more button poc #12563
Read more button poc #12563
Conversation
…ad-more-button-poc
…ad-more-button-poc
…the chosen services
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -225,6 +225,14 @@ | |||
| /> | |||
| ); | |||
|
|
|||
| export const ArticlePageWithReadMoreButtonVariation1 = () => ( | |||
| <ComponentWithServiceContext data={articleData} service="pidgin" /> | |||
Check failure
Code scanning / CodeQL
Hard-coded credentials Critical
| ); | ||
|
|
||
| export const ArticlePageWithReadMoreButtonVariation2 = () => ( | ||
| <ComponentWithServiceContext data={articleData} service="mundo" /> |
Check failure
Code scanning / CodeQL
Hard-coded credentials Critical
| @@ -11,7 +11,7 @@ export default { | |||
| display: 'block', | |||
| width: 'calc(100% - 2rem)', | |||
| padding: `${spacings.DOUBLE}rem 0`, | |||
| margin: `${spacings.QUADRUPLE}rem ${spacings.DOUBLE}rem -0.5rem ${spacings.DOUBLE}rem`, | |||
| margin: `${spacings.QUADRUPLE}rem ${spacings.DOUBLE}rem -1.5rem ${spacings.DOUBLE}rem`, | |||
There was a problem hiding this comment.
The bottom of the <main> container, which contains the button, has a padding of 24px. The <aside> underneath which contains the related content component has a top margin of 16px. The Related Content section has a margin top of 16px. To achieve the gap of 32px between the button and the related content component heading in the designs I have added a minus margin of 1.5rem.
Is it always the related content heading first? Do all headings have this margin top?
…ad-more-button-poc
…ad-more-button-poc
…ad-more-button-poc
There was a problem hiding this comment.
So far so good 😄
- Great decision to add the button at the bottom of the article text - that way if No CSS then it doens’t interrupt the reading experience
- Know this is still in progress, looks like a little less space above the button in the design for both variants?
- Also may need something for forced colours…
| marginLeft: '0', | ||
| verticalAlign: 'middle', | ||
| }} | ||
| aria-hidden="true" |
There was a problem hiding this comment.
Can we add focusable false too please?
| css={{ | ||
| marginRight: '10px', | ||
| marginLeft: '0', | ||
| verticalAlign: 'middle', |
There was a problem hiding this comment.
Think we need to try to get it to align with the center of the button text
| width="16" | ||
| height={theme.fontSizes.pica.fontSize} | ||
| css={{ | ||
| marginRight: '10px', |
| @@ -68,6 +68,7 @@ import Disclaimer from '../../components/Disclaimer'; | |||
| import SecondaryColumn from './SecondaryColumn'; | |||
| import styles from './ArticlePage.styles'; | |||
| import { ComponentToRenderProps, TimeStampProps } from './types'; | |||
| import ReadMoreButton from './ReadMoreButton'; | |||
There was a problem hiding this comment.
Not sure if we should call this "read more" how about 'show more' as that is what we are doing??
There was a problem hiding this comment.
Actually now below I see you have used the naming 'show' so maybe this is a better name?
| border: 'none', | ||
|
|
||
| '&:hover, &:focus': { | ||
| textDecoration: 'underline 2px', |
There was a problem hiding this comment.
Can we use relative values please? Also not sure we usually specify the width when it's text underline (just when we use border)
| color: '#141414', | ||
| textAlign: 'left', | ||
| border: 'none', | ||
| borderBottom: '1px solid #B0B2B4', |
| margin: `${spacings.TRIPLE}rem ${spacings.DOUBLE}rem -0.5rem ${spacings.DOUBLE}rem`, | ||
| backgroundColor: '#F6F6F6', | ||
| color: '#141414', | ||
| textAlign: 'left', |
| paddingBottom: `calc(${spacings.DOUBLE}rem + 0.5rem)`, | ||
|
|
||
| '&:hover, &:focus': { | ||
| textDecoration: 'underline 2px', |
There was a problem hiding this comment.
Can we use relative values please? Also not sure we usually specify the width when it's text underline (just when we use border)
Resolves JIRA:
Summary
A very high-level summary of easily-reproducible changes that can be understood by non-devs, and why these changes where made.
Code changes
Developer Checklist
Testing
Ready-For-Test, Local)Ready-For-Test, Test)Ready-For-Test, Preview)Ready-For-Test, Live)Additional Testing Steps
Useful Links