Skip to content

Refactor about page v2#760

Merged
distalx merged 34 commits into
codebuddies:stagingfrom
railsstudent:refactor-about-page-v2
Jan 12, 2018
Merged

Refactor about page v2#760
distalx merged 34 commits into
codebuddies:stagingfrom
railsstudent:refactor-about-page-v2

Conversation

@railsstudent

Copy link
Copy Markdown
Collaborator

Externalize details of contributors in private/about.json and read the data from it when rendering the images and popups in about.html.

When new contributors wants to add themselves to about page, they can append JSON object of personal details in code-contributors array.

The html code in title attribute in original design is errone hone with all these angle brackets

distalx and others added 29 commits November 8, 2017 21:52
@railsstudent railsstudent requested a review from lpatmo January 6, 2018 06:13
@railsstudent railsstudent requested a review from distalx January 6, 2018 06:13
@lpatmo lpatmo added the [state] in-review the implementation that addresses this issue is up for review label Jan 6, 2018

@lpatmo lpatmo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! @railsstudent Could I move <img src="iframe.php?url=https%3A%2F%2Favatars1.githubusercontent.com%2Fu%2F1159649%3Fv%3D4%26amp%3Bs%3D460" class="img-circle" alt="Connie Leung"/> up to the core contributors section? Will link to your twitter and github.

@railsstudent

Copy link
Copy Markdown
Collaborator Author

@lpatmo Yes, you can move my image to core contributor session.
My slack handler name is @connie
twitter is con_leung
github is railsstudent

Thanks.

@distalx distalx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Here are some suggestions I hope you'll find helpful.

Comment thread private/about.json Outdated
@@ -0,0 +1,235 @@
{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should move this file under imports/data directory, and break it down to two files,
The scope of the private directory is limited to server side only.

very roughly :

imports/data/flags.json and imports/data/contributors.json

[
  {
     name: 'xyz',
     avatar: 'http://link or gravater',
     intro: 'lorem ipsum',
     socials:[
         {website: 'URL'},
         {twitter: '@handel'},
     ....
     ],
     type: 'code'
  },
   ....
]

on about page you can load this json files with their relative path

imports contributors from 'imports/data/contributors.json'

with the help of the helper function contributors can be passed to the template

`contributors : function() { return contributors }`

and then we can iterate over it using
{{#each contributors}} {{/each}} block.

To display contributors into their respective section we can use equals template helper {{#if equals type 'code'}}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see imports/data directory in the project. Should I create it? thanks.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, imports directory is a part of a new directory structure suggested in meteor guide, our migration to the new directory structure is long over due.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted. Will get this done before Sunday.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@distalx Please kindly review

railsstudent and others added 5 commits January 12, 2018 00:00
@distalx distalx merged commit 386c1c2 into codebuddies:staging Jan 12, 2018
@lpatmo lpatmo added [state] closed the issue is now closed, see comments for more information and removed [state] in-review the implementation that addresses this issue is up for review labels Jan 12, 2018
@distalx

distalx commented Jan 12, 2018

Copy link
Copy Markdown
Member

@railsstudent , Thanks for the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[state] closed the issue is now closed, see comments for more information

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants