feat: Add announcements to news page #724
Conversation
|
This pull request has been linked to Shortcut Story #588: Add Announcements to News & Announcements page. |
jcbcapps
changed the title
| render(<ArticleListItem article={testArticle} />) | ||
| it('renders the article preview of a cms article', () => { | ||
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore |
There was a problem hiding this comment.
curious what we're ignoring with this?
There was a problem hiding this comment.
In the LabelRecord type, there is a field named type and it's type (I know...confusing) is defined as 'Source' | 'Audience' | 'Base'. In the mock article for this file, I gave the type field a value of Source but it kept giving me an error saying that it doesn't match the type, but it does. Not sure why it wouldn't accept it, so I figured it would be fine to ignore the line since it's a test.
There was a problem hiding this comment.
It looks like if you add the type ArticleListItemRecord when defining the const cmsTestArticle it will recognize the SearchResultType correct.
const cmsTestArticle: ArticleListItemRecord = {
id: 'testArticleId123',
labels: [{ id: 'labelId', name: 'A Label', type: 'Source' }],
...
}
There was a problem hiding this comment.
You are correct. Fixed it!
| @@ -6,12 +6,14 @@ describe('ORBIT Blog', () => { | |||
| beforeEach(() => { | |||
| cy.preserveLoginCookies() | |||
| cy.visit('/') | |||
| cy.injectAxe() | |||
There was a problem hiding this comment.
thought: I wonder if visit could be made to do this in addition to visiting the url. Though if we switch to playwright this might be moot.
| // Slider component in react-slick clones each item in the carousel, | ||
| // so a length of 2 is accurate | ||
| expect(screen.getAllByText('Test Announcement')).toHaveLength(2) |
There was a problem hiding this comment.
praise: thanks for including the note again here! This is something I'd find confusing without one :)
|
|
||
| const RSS_URL = `${SPACEFORCE_NEWS_RSS_URL}&max=12` | ||
|
|
||
| const NewsAnnouncements = ({ |
There was a problem hiding this comment.
Question: This PR is dependent on PR #723 correct?
There was a problem hiding this comment.
Correct. I forgot to mention that in this PR. I'll be sure to merge that one first.
SC-588
Proposed changes
Add the announcement component to the
/news-announcementspageReviewer notes
With published announcements in your local CMS, click the
Newslink in the header:/news-announcementsSetup
Make sure to have both the CMS and the portal client running on your local machine. You can do this by
yarn services:up && yarn devin the CMS repo andyarn devin the portal client with this branch checked outAlso, make sure that you have some published announcements in your local CMS
Code review steps
As the original developer, I have
As code reviewer(s), I have
As a designer reviewer, I have
As a test user, I have
Screenshots