-
Notifications
You must be signed in to change notification settings - Fork 1
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just a few comments below
import { LinkPost } from '../components/Blog/LinkPost' | ||
import { PodcastListItem } from '../components/Blog/PodcastListItem' | ||
import { PodcastPost } from '../components/Blog/PodcastPost' | ||
import { PressReleaseListItem } from '../components/Blog/PressReleaseListItem' | ||
import { PressReleasePost } from '../components/Blog/PressReleasePost' | ||
import { ReleasePost } from '../components/Blog/ReleasePost' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the @component
decorator be used to combine these imports?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this was before we had aliases added to our tsconfig
, but I agree! I think we can revise all of these under the '@components'
alias 🤓
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I totally agree. I attempted to do this, and ran into an issue when incorporating the alias. The current state is here: 011165f. I spent some time examining this, but couldn't pin down what I missed. Here is what I noticed:
- Components don't appear to import when
from '../components/Blog'
is changed to@components
PressReleaseListItem
returns an export error when it is included in the group, although it appears to be exported in the proper places
I appreciate any guidance on what I may be missing here. I may have been looking at this for too long 😅
While doing this, I saw that we have two components named BlogListItem
for distinct purposes. One is used as a preview for blogposts on /blog
while the other is used to link users to resources from pages such as Code Insights and the Use Cases. To differentiate them, I renamed the latter BlogResourceItem
throughout.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My hunch is that it's something fishy with the enums in posts.ts
. I'll take a closer look tomorrow! Shouldn't be blocking to put this PR through for now though. Thanks for changing that dupe file name too!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, Becca. It looks like this was interfering with the build, so I have reverted the imports back to how they were to meet our tests. I will look into this more, too--happy to collaborate on figuring this one out! 550b560
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed the latter
BlogResourceItem
throughout.
Awesome!! Good eye on that and thanks for that @zlonko!!
It seems like it may be a config issue because I don't get linting errors when changing to @components
, but the components aren't resolving when they're another level deep. We can look back into this later and merge for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work @zlonko! Just a few questions/revisions:
import { LinkPost } from '../components/Blog/LinkPost' | ||
import { PodcastListItem } from '../components/Blog/PodcastListItem' | ||
import { PodcastPost } from '../components/Blog/PodcastPost' | ||
import { PressReleaseListItem } from '../components/Blog/PressReleaseListItem' | ||
import { PressReleasePost } from '../components/Blog/PressReleasePost' | ||
import { ReleasePost } from '../components/Blog/ReleasePost' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this was before we had aliases added to our tsconfig
, but I agree! I think we can revise all of these under the '@components'
alias 🤓
✅ Deploy Preview for about-replatform ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
This closes #94. It brings over the press release page and accompanying markdown files.
Notes
PressReleasePost
andPressReleaseListItem
press-release
directory aspress
for consistency with the "press" tag and to avoid array statement ingetStaticProps
.max-h-
in `_spacing.scssTesting
/press-release