[improve/feat][site] Layout and content updates to Homepage, Features, Community, Events, Books and Blog, Added Use Cases Page. Resources are split into Articles and Presentations Pages. #789
Conversation
|
@Trakina Please add the following content to your PR description and select a checkbox: |
data/usecases.ts
Outdated
| smalltext: string; | ||
| docslink: string; | ||
| caselink: boolean; |
There was a problem hiding this comment.
| smalltext: string; | |
| docslink: string; | |
| caselink: boolean; | |
| smallText: string; | |
| docsLink: string; | |
| caseLink: boolean; |
data/usecases.ts
Outdated
| docslink: string; | ||
| caselink: boolean; | ||
| }; | ||
| const usecases: UseCase[] = [ |
There was a problem hiding this comment.
| const usecases: UseCase[] = [ | |
| const useCases: UseCase[] = [ |
data/usecases.ts
Outdated
| import React, { ReactNode } from 'react'; | ||
|
|
||
| export type UseCase = { | ||
| icon: string; |
There was a problem hiding this comment.
Instead of this comment above:
// possible icons, so far: 'user', 'arrow', 'speech' and 'check'
we can use union type here
icon: 'user' | 'arrow' | 'speech' | 'check';
| <a className={s.ForumLink} target="_blank" href={props.forum_link}><strong>{props.forum}</strong></a> | ||
| </div> | ||
| {props.date && <div className={s.Date}>{props.date}</div>} | ||
| {props.tags && <div className={s.Tags}>{tagsArray.map((tag)=>( <small>{tag}</small> ))}</div>} |
There was a problem hiding this comment.
React key property is missed here.
|
|
||
| const Card: React.FC<data.Resource> = (props) => { | ||
| let tagsArray = new Array(); | ||
| if(props.tags) tagsArray = props.tags.split(", "); |
There was a problem hiding this comment.
It probably makes sense to change the Resource['tags'] type from string to string[].
| @@ -14,6 +13,15 @@ const CaseStudiesPage: React.FC = () => { | |||
| const [searchQuery, setSearchQuery] = useState(''); | |||
| const [categoryFilter, setCategoryFilter] = React.useState<CategoryFilterOption>('any'); | |||
|
|
|||
| const updateCategoryLinks = (newoption) => { | |||
There was a problem hiding this comment.
newoption => newOption
allfilterlinks => allFilterLinks
| setCategoryFilter(newoption); | ||
| let allfilterlinks = document.querySelectorAll('.'+s.CategoryFilterLink); | ||
| allfilterlinks.forEach((el) => { | ||
| if(el.dataset.option == newoption) el.classList.add(s.active); |
There was a problem hiding this comment.
Maybe reformat it to the more commonly used if/else statement formatting.
if (smth) {
...
} else {
...
}
| <div className={s.FiltersMobile}> | ||
| <div> | ||
| {categoryFilterOptions.map((option) => ( | ||
| <button type="button" data-option={option} onClick={() => updateCategoryLinks(option)} className={s.CategoryFilterLink+(option === 'any' ? ' '+s.active : '')}>{option === 'any' ? 'All Industries' : data.categoryLabels[option]}</button> |
There was a problem hiding this comment.
data-option and the document.querySelectorAll above aren't the React way to do it, but ok if it works as expected.
| return ( | ||
| <div> | ||
| {props.data.map((row, index) => ( | ||
| <div> |
| id: "subscribe", | ||
| text: "Subscribe", | ||
| href: "mailto:[email protected]", | ||
| /*{ |
There was a problem hiding this comment.
Do we need the commented code here?
| }, | ||
| /*{ |
There was a problem hiding this comment.
Do we need the commented code here?
| ), | ||
| actions: [ | ||
| { | ||
| id: "contributing-to-the-project", | ||
| href: useBaseUrl("/contribute"), | ||
| text: "Contribution guide", | ||
| type: "primary", | ||
| type: "transparentblack", |
There was a problem hiding this comment.
transparentblack => transparentBlack"
| ), | ||
| /* |
There was a problem hiding this comment.
Do we need the commented code here?
| @@ -6,12 +6,13 @@ type ActionButtonProps = { | |||
| id: string; | |||
| text: string; | |||
| href: string; | |||
| type: "primary" | "normal" | "link"; | |||
| type: "primary" | "normal" | "link" | "transparentwhite" | "transparentblack"; | |||
There was a problem hiding this comment.
transparentwhite => transparentWhite
transparentblack => transparentBlack
| @@ -21,17 +22,19 @@ export type ContentCardProps = { | |||
| }; | |||
|
|
|||
| const ContentCard: React.FC<ContentCardProps> = (props) => { | |||
| let theclass = s.ContentCard; | |||
| if(props.format == 'column') theclass = s.ContentCardColumn; | |||
There was a problem hiding this comment.
if(smth) do
=>
if (smth) {
do
}
|
I synced the deployment with the latest changes: https://pulsar-site-2024-updates.vercel.app/ |
jianghaiting => Jason918 
Design Implementation 
|
| <Button title={isShowMoreCmtrs ? 'Show less' : 'Show more'} variant="transparentWhite" onClick={() => setIsShowMoreCmtrs(!isShowMoreCmtrs)}/> | ||
| ); | ||
|
|
||
| const forbiddenUsers = ['jianghaiting', 'technoboy', 'linlin'] |
There was a problem hiding this comment.
This piece of code looks a bit odd to me.
Could we just add an optional githubId field here:
Line 5 in aaaffa8
Probably make it optional and take githubId by default with a fallback to apacheId.
|
@Trakina thank you for the update.
|
| element.apacheId = currectUsers[forbiddenUsers.indexOf(element.apacheId)]; | ||
| } | ||
| return element | ||
| type apacheId = string |
There was a problem hiding this comment.
| type apacheId = string | |
| type ApacheId = string |
The common TypeScript convention is to use capitalized CamelCase for type names.
|
@Trakina I think we're steadily getting closer to the "good enough" state. 🙂 I noted the most important issues that should be fixed before merging the PR. All the other little fixes can take much time. We should decide what level of polishing we want to achieve in this PR. cc @asafm Preview link: https://pulsar-site-2024-updates.vercel.app
|
|
We aim for high quality |
|
@asafm ok ;) Then I have a few more comments for the "Community Page":
Design Implementation 
Implementation
@Trakina you can double-click any Figma element to get the desired font size and compare it with font size at the computed styles tab in your browser devtools. Could you please ensure that font sizes and paddings are correct across the other pages and make the corresponding fixes? |
|
@visortelle Wow, thanks for the high attention paid to the details review!! 💯 |
I wonder why we use Slack at all for anything except 1 to 1 direct messages. |
|
@Trakina thank you for the updates! Figma design: https://www.figma.com/file/0jERoA2DTlfQoj1uH09FxQ/Pulsar-website
I didn't re-check on the mobile and tablet. I think it's enough to fix. |
|
@Trakina it would probably make sense to mark the comment checkboxes as done when it ready. If you can't edit my comment, copy-paste my comment and mark what is done. Copy paste it as a new commentAlthough the right way would be if I'd made a separate comment for each issue that could be marked as resolved, but it's' too late. |
|
We should probably update the mailing list links on the community page according to this PR: |
@visortelle I just copied the new images and updated the header styles. The rest of it wasn't on Figma, so I did not do it. I didn't re-check on the mobile and tablet. I think it's enough to fix. |
|
@Trakina Thank you. I updated the live preview: https://pulsar-site-2024-updates.vercel.app/ I’m going to take a look and commit a bunch of minor fixes for the desktop version in the next few days. Could you add me as a collaborator to your repo fork? |
|
@visortelle just realised I missed a detail in the blog list, pushed again. |
|
@Trakina @visortelle @asafm I'll push a merge conflict resolution so that we get this merged asap. Hopefully I'm making the correct choices. |
|
@lhotari agree, we can apply other fixes in further PRs. |
This PR updates the layouts and content to the Homepage, Features, Community, Events, Books and Blog, adds Use Cases Page and splits Resources into Articles and Presentations Pages.
docdoc-requireddoc-not-neededdoc-complete