New design #31

Merged
Madeorsk merged 8 commits from master into master 6 years ago
Madeorsk commented 6 years ago (Migrated from github.com)
  • New design!
  • Added link for author name in comments.
  • Added author full name in comments.
  • Fixed label / input relation linking.
+ New design! + Added link for author name in comments. + Added author full name in comments. * Fixed label / input relation linking.
mkljczk commented 6 years ago (Migrated from github.com)

screenshot
if anyone's curious how does it look

![screenshot](https://user-images.githubusercontent.com/21127288/40318526-73f79c7e-5d14-11e8-877d-a57baf201e47.png) if anyone's curious how does it look
elegaanz (Migrated from github.com) requested changes 6 years ago
elegaanz (Migrated from github.com) left a comment

Hey! Thanks for your help! I just have a few remarks:

  • the background seems a bit dark to me. I'm not against a light-gray background, but it should be more subtle.
  • I don't know if it is wanted or not, but on lists (either posts, notifications or followers) the title is not perfectly left-aligned with the others elements of the card.
  • There is a weird spacing between comments when you have many of them.
  • The button to edit your own profile looks weird
  • About the style of the code, I would prefer to use One True Brace Style everywhere (but it's not really important, I can do the formatting myself after merging if you want).
  • The header seems a bit big to me. I think the content itself (especially when reading articles) should be the main element on the screen, so it doesn't really makes sense to me to have big links here.
  • I'm not sure it perfectly responsive, but that is not the case with the current design neither, so it's not a real problem, and it can completely be fixed in another PR.

Else, I really love what you did (especially the fonts you chose). Thanks again!

Hey! Thanks for your help! I just have a few remarks: - the background seems a bit dark to me. I'm not against a light-gray background, but it should be more subtle. - I don't know if it is wanted or not, but on lists (either posts, notifications or followers) the title is not perfectly left-aligned with the others elements of the card. - There is a weird spacing between comments when you have many of them. - The button to edit your own profile looks weird - About the style of the code, I would prefer to use One True Brace Style everywhere (but it's not really important, I can do the formatting myself after merging if you want). - The header seems a bit big to me. I think the content itself (especially when reading articles) should be the main element on the screen, so it doesn't really makes sense to me to have big links here. - I'm not sure it perfectly responsive, but that is not the case with the current design neither, so it's not a real problem, and it can completely be fixed in another PR. Else, I really love what you did (especially the fonts you chose). Thanks again!
EliotBerriot (Migrated from github.com) reviewed 6 years ago
EliotBerriot (Migrated from github.com) left a comment

Looks good!

One remark: the number of characters per line seems too high. I suggest setting a max-width for text content to increase the readability (see https://baymard.com/blog/line-length-readability, for an explanation)

Looks good! One remark: the number of characters per line seems too high. I suggest setting a max-width for text content to increase the readability (see https://baymard.com/blog/line-length-readability, for an explanation)
stephenburgess8 commented 6 years ago (Migrated from github.com)

Hey, just wanted to say this looks great. I also wanted to echo Eliot and Baptiste on two points: line length and contrast. For line length, my go-to number is ~66 characters per line. That comes from The Elements of Typographic Style by Robert Bringhurst. He says comfortable reading line length is between 45 and 77 characters, with about 66 being ideal. The blog post above suggests 50-60, but I think you can get away with a little longer lines in long form posts.

The other thing is text contrast on background. I wrote a blog post about this including samples for minimum contrast. https://webdev.ink/2018/04/24/Text-Contrast-for-Web-Pages/ Basically, a 7:1 ratio of text to background color is the minimum to meet WCAG AAA standards and ensure that everyone can use the site easily. Looking forward to the next iteration!

Hey, just wanted to say this looks great. I also wanted to echo Eliot and Baptiste on two points: line length and contrast. For line length, my go-to number is ~66 characters per line. That comes from **The Elements of Typographic Style** by Robert Bringhurst. He says comfortable reading line length is between 45 and 77 characters, with about 66 being ideal. The blog post above suggests 50-60, but I think you can get away with a little longer lines in long form posts. The other thing is text contrast on background. I wrote a blog post about this including samples for minimum contrast. https://webdev.ink/2018/04/24/Text-Contrast-for-Web-Pages/ Basically, a 7:1 ratio of text to background color is the minimum to meet WCAG AAA standards and ensure that everyone can use the site easily. Looking forward to the next iteration!
sivy commented 6 years ago (Migrated from github.com)

I hope that no one will mind me making a few comments:

  • As much as I do not like aping another site's style, taking design "inspiration" from Medium could be a Good Thing, since it's something users are familiar with
  • I've been wondering if Plume was intended as a Medium-alike (a pattern we should get away from in the Fediverse) or a more general Blog provider. This is important, since blogs are such a personal medium (no pun intended) and I'm wondering if you are intending to give users some control over their blog's design.
I hope that no one will mind me making a few comments: - As much as I do not like aping another site's style, taking design "inspiration" from Medium could be a Good Thing, since it's something users are familiar with - I've been wondering if Plume was intended as a Medium-alike (a pattern we should get away from in the Fediverse) or a more general Blog provider. This is important, since blogs are such a personal medium (no pun intended) and I'm wondering if you are intending to give users some control over their blog's design.
elegaanz commented 6 years ago (Migrated from github.com)

We plan to support theming, both for instances and blogs (see #40)

We plan to support theming, both for instances and blogs (see #40)
Madeorsk commented 6 years ago (Migrated from github.com)

I tried to take every observations in account in my last commit. What do you think about it, now? :-)

(There are still Google Fonts in main.css that I would like to download and put into the fonts directory, I'll try to do this tomorrow.)

I am thinking about some improvements, especially for the dashboard, user profile, lists (followers, notifications), forms (login, register, ...), and like/reshare buttons. But I think it could come in future PR!

I tried to take every observations in account in my last commit. What do you think about it, now? :-) (There are still Google Fonts in main.css that I would like to download and put into the `fonts` directory, I'll try to do this tomorrow.) I am thinking about some improvements, especially for the dashboard, user profile, lists (followers, notifications), forms (login, register, ...), and like/reshare buttons. But I think it could come in future PR!
elegaanz (Migrated from github.com) requested changes 6 years ago
elegaanz (Migrated from github.com) left a comment

It's better, but I still have a few remarks:

  • the top margin of the header still seems big too me (I tested locally and margin-top: 3em looks better IMHO).
  • The header feels a bit unbalanced because of the "Plume" part. I think it could be improved by reducing its right padding.
  • Maybe a larger margin between the article itself and its metadata/comments?
  • I don't remember if I already said it, but items in lists are not perfectly aligned with other items on the left (it is especially visible on the dashboard)

Else, it looks really good. I hope I'm not too demanding, and as usual if you think something I said is not legitimate, feel free to tell me.

It's better, but I still have a few remarks: - the top margin of the header still seems big too me (I tested locally and `margin-top: 3em` looks better IMHO). - The header feels a bit unbalanced because of the "Plume" part. I think it could be improved by reducing its right padding. - Maybe a larger margin between the article itself and its metadata/comments? - I don't remember if I already said it, but items in lists are not perfectly aligned with other items on the left (it is especially visible on the dashboard) Else, it looks really good. I hope I'm not too demanding, and as usual if you think something I said is not legitimate, feel free to tell me.
elegaanz commented 6 years ago (Migrated from github.com)

Here are screenshots of the updated design for those who are curious:

capture d ecran de 2018-06-17 21-48-21

capture d ecran de 2018-06-17 21-48-44

Here are screenshots of the updated design for those who are curious: ![capture d ecran de 2018-06-17 21-48-21](https://user-images.githubusercontent.com/16254623/41511927-4e20c3e4-7278-11e8-9446-a9978edff431.png) ![capture d ecran de 2018-06-17 21-48-44](https://user-images.githubusercontent.com/16254623/41511928-4f259030-7278-11e8-9ef4-5438a216d054.png)
elegaanz commented 6 years ago (Migrated from github.com)

So, once again screenshots with the latest commits for those who don't want to install a development environment.

capture d ecran de 2018-06-19 22-41-58

(I personally love this new header 👌)

So, once again screenshots with the latest commits for those who don't want to install a development environment. ![capture d ecran de 2018-06-19 22-41-58](https://user-images.githubusercontent.com/16254623/41625835-2dc98b30-7412-11e8-883f-8cfabdace50e.png) (I personally love this new header :ok_hand:)
elegaanz commented 6 years ago (Migrated from github.com)

@Madeorsk I don't know if you plan to make other changes to this branch but I'm okay to merge it as it is id you want, even if some minor improvements could be made (it will never be too late to make them in other branches). (the diff is a bit huge tho, but I guess it's because of the fonts)

@Madeorsk I don't know if you plan to make other changes to this branch but I'm okay to merge it as it is id you want, even if some minor improvements could be made (it will never be too late to make them in other branches). (the diff is a bit huge tho, but I guess it's because of the fonts)
mkljczk commented 6 years ago (Migrated from github.com)

This new header is great 😍

This new header is great 😍
Madeorsk commented 6 years ago (Migrated from github.com)

I think you can merge!

Yes there is still a lot of things to do, like I said I want to change dashboard, followers list (we maybe need to add more informations than a name, same for the dashboard), like/reshare buttons and forms, at least.

I think you can merge! Yes there is still a lot of things to do, like I said I want to change dashboard, followers list (we maybe need to add more informations than a name, same for the dashboard), like/reshare buttons and forms, at least.
elegaanz commented 6 years ago (Migrated from github.com)

Thank you so much!

Thank you so much!

Reviewers

The pull request has been merged as 41792b3c19.
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b master master
git pull origin master

Step 2:

Merge the changes and update on Forgejo.
git checkout master
git merge --no-ff master
git push origin master
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: Plume/Plume#31
Loading…
There is no content yet.