Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementation of User Profile Dashboard #39

Merged
merged 11 commits into from
Nov 2, 2018

Conversation

creatrixity
Copy link
Contributor

@creatrixity creatrixity commented Nov 2, 2018

knacksteem-wip-3

This pull adds the user profile dashboard.

To have this feature fully implemented, moderation (make mod/remove mod) and account access (ban/unban) functionality must be integrated. This will be included in the next pull.

Also more comprehensive empty states should be added. Currently, only posts on the database are displayed. Maybe have them fetch from the blockchain perhaps?

References issue #35

@@ -0,0 +1,1028 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

Only use yarn remove this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How do I get rid of this?

@@ -7,6 +7,7 @@
"axios": "^0.18.0",
"express": "^4.16.3",
"express-http-to-https": "^1.1.4",
"fecha": "^2.3.3",
Copy link
Member

Choose a reason for hiding this comment

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

Where did you use this dependency

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi. I plan to use fecha to parse date on the Profile page. It's really lightweight and at 3kb gzipped it's a better alternative to Moment.js

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you have a better/more efficient way of parsing dates, please let me know.

Copy link
Member

Choose a reason for hiding this comment

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

Actually I saw this PR so i judged it that way it would be better if you introduced it just as you used it otherwise for this PR it looks like a unused dependency 🙂

You can keep it if you are going to use it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm currently using it for the next pull.

@@ -0,0 +1,3 @@
import ProfileCategoriesBar from './ProfileCategoriesBar';
Copy link
Member

Choose a reason for hiding this comment

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

This file just imports and exports another module we can simply import the module as it is just one module

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I use the Component Folder Pattern. It helps make code more searchable. However if the convention is against this, I will revert.

@@ -0,0 +1,3 @@
import ProfileHero from './ProfileHero';
Copy link
Member

Choose a reason for hiding this comment

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

Same here

@@ -0,0 +1,3 @@
import ProfileInfoBar from './ProfileInfoBar';
Copy link
Member

Choose a reason for hiding this comment

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

Same here

@@ -0,0 +1,3 @@
import ProfileMetaBar from './ProfileMetaBar';
Copy link
Member

Choose a reason for hiding this comment

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

⬆️

@@ -0,0 +1,3 @@
import Profile from './Profile';
Copy link
Member

Choose a reason for hiding this comment

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

⬆️

Copy link
Member

@ms10398 ms10398 left a comment

Choose a reason for hiding this comment

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

I have done the code review

I would do functional review in a while can you do the required changes 🙂

@ms10398
Copy link
Member

ms10398 commented Nov 2, 2018

Okay I was performing the functional review

Overall the UI looks good

screenshot from 2018-11-02 17-37-23

These are not properly aligned

If there is no name of the account the UI breaks see below
screenshot from 2018-11-02 17-42-28

Would be cool if the icon in the header after login leads to knacksteem profile instead of steemit profile.

and the tabs Followers and Following is not working I suppose that is not done in the PR and would be done in another PR.

Rest LGTM 👍

you can do the changes suggested above in the code and it is good to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants