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

Visual improvements 3.0 #2978

Closed
ineagu opened this issue Jul 23, 2021 · 18 comments
Closed

Visual improvements 3.0 #2978

ineagu opened this issue Jul 23, 2021 · 18 comments
Assignees
Labels
bug This label could be used to identify issues that are caused by a defect in the product. released Indicate that an issue has been resolved and released in a particular version of the product.

Comments

@ineagu
Copy link

ineagu commented Jul 23, 2021

Description:

Some of those might be old issues, but since they are related to the frontend, I would say that if we don't fix them now while we do the 2 styles part, it would be even harder further ( if we want to fix those).

In order to test I imported some articles from codeinwp blog and used several other themes to compare.

  1. https://vertis.d.pr/pagLbK - there is no bottom margin/padding on ul and no top margin/padding to H4, which cause things to look weird when they come one after another. Neve - https://vertis.d.pr/pagLbK, Kadence - https://vertis.d.pr/pagLbK, article is: https://www.codeinwp.com/blog/best-web-hosting-for-small-business/ .

  2. Meta spacing for me looks weird, I won't use.

This might be personal choice, since I mentioned it before but everyone was ok. Here is Kadence: https://vertis.d.pr/bAQHCU, Here is Neve: https://vertis.d.pr/7u7k6P ( title still is a bit too big for long titles)

  1. Default H2 is still big when you have long H2 in content: https://vertis.d.pr/7u7k6P.

  2. Categories aren't looking great, maybe we can tweak the defaults.

Don't we display excerpts by default? here is Neve: https://vertis.d.pr/ZUvOqq vs Kadence: https://vertis.d.pr/ZUvOqq

  1. There are various little things that make Kadence look better in terms of content( no color on links in H, different spacing around various elements, underlined , spacing after H4 etc). I am no expert to say what.

  2. No separation between posts on archive.

  3. Neither Astra nor Kadence display the category descriptions within the menu like we do : https://vertis.d.pr/F3u998

  4. Author pages aren't great neither

Neve: https://vertis.d.pr/Jqx4ls, Blocksy: https://vertis.d.pr/CXaHPN

We can look at author/category pages on themeisle/codeinwp : https://www.codeinwp.com/blog/category/productivity-tips/, https://vertis.d.pr/QHjIWg

  1. I would say that pages should not have the sidebar enabled by default.

10, Fonts seems small within tables due to 0.9em rule: https://vertis.d.pr/Y4tfze

@gutoslv I am not sure if is a good idea, but I would find real world sites/content and test how the release look like on those. I think 3.0 is a special version, since we can introduce breaking changes, for the better, due to the fact that the old user won't automatically get the new version, so we should make the most out of this.

@JohnPixle some might not be relevant, but might worth taking a look and diving deeper for the reasons mentioned above, feel free to see with @selul what make sense to do moving forward.

@gutoslv
Copy link
Contributor

gutoslv commented Jul 23, 2021

@ineagu, I think the idea of testing it on real-world sites and content is a good idea; currently, I'm testing with the starter sites, which I found is the closest I can get to a real-world site since I don't have access to any real-world site using WordPress. How may I get access to our blogs, at least to import the posts?

Taking this real-world situation, for the next big releases would be a good idea to release an alpha or beta version to selected users in order to have feedback from end-users. What do you think about this?

@ineagu
Copy link
Author

ineagu commented Jul 24, 2021

@ineagu, I think the idea of testing it on real-world sites and content is a good idea; currently, I'm testing with the starter sites, which I found is the closest I can get to a real-world site since I don't have access to any real-world site using WordPress. How may I get access to our blogs, at least to import the posts?

Taking this real-world situation, for the next big releases would be a good idea to release an alpha or beta version to selected users in order to have feedback from end-users. What do you think about this?

Feel free to sync with @selul on that, an idea that I had was to use the staging for our own sites to test theme update ( we use neve on codeinwp/themeisle/optimole) + theme switching and comparison with various other themes.

Of course having some customers to beta-test or give us access to some staging woudl be awesome, I think I talked with Marius some long time ago.

@JohnPixle
Copy link

JohnPixle commented Jul 26, 2021

Thanks @ineagu for the input.

1. there is no bottom margin/padding on ul and no top margin/padding to H4
Good catch, we should make sure that all the list elements have appropriate bottom margin.
Overall I think we should be adding bottom margins (instead of top margins to Hs).

2. Meta spacing for me looks weird, I won't use.
I agree that we can lower the default margins there, but this is part of a broader concern.
As long as we have a fixed value for margins of these elements (post title / post meta), we will have this problem.

I have proposed a solution for this, which is to add bottom margin controls for these elements under the blog typography.

I don't know if we have the bandwidth to approve this solution and implement it right away in the version, but we can safely reduce the default spacing for now. I'll follow up with @abaicus .

3. Default H2 is still big when you have long H2 in content
We discussed internally, and decided to downscale eventually. Will be following up with Andrei as well with new sizes.
I'll name the file type-scale-final2_new_FINAL_v55_FINAL 🤓

4. Categories aren't looking great, maybe we can tweak the defaults.
I see the same screenshot for Neve: https://vertis.d.pr/ZUvOqq and Kadence: https://vertis.d.pr/ZUvOqq, but I get the point. Following up with Andrei as well.

5. There are various little things that make Kadence look better in terms of content
Not for long! 👩🏻‍🎤

6. No separation between posts on archive.
Will follow up as well, depending on what the new default archive layout will be.

7. Neither Astra nor Kadence display the category descriptions within the menu like we do
We shouldn't do that, no question :)

9. I would say that pages should not have the sidebar enabled by default.
Yes, they can go away. Right after the release we plan to work on a new default demo anyway, taking advantage of all the new features. The pages will shape up nicely.

10. Fonts seems small within tables due to 0.9em rule:
I don't think they do, but I will follow up with @abaicus with this if needed.
I am not seeing this table with the new Neve 3 styles btw, maybe it has some custom css on it?

@JohnPixle
Copy link

Sorry, accidentally hit the comment button without finishing :(
I will continue editing the post above.

@selul
Copy link
Contributor

selul commented Jul 26, 2021

@JohnPixle will need your thoughts on the next points and if you see any improvements that we can add on the theme end for 3, 5, 6, 8, 10.

@abaicus for 4, 7, 9 do you have any thoughts, I agree with them and I thought that we already do them.

@selul
Copy link
Contributor

selul commented Jul 26, 2021

@ineagu can you review your screenshots from 4 ? None of us understand your point there.

@ineagu
Copy link
Author

ineagu commented Jul 26, 2021

@selul we can just say that the category page looks bad, this is how it looks: https://vertis.d.pr/ZUvOqq ( no separation between posts, between page and the rest).

Here is how a better version looks like: https://wp-themes.com/kadence/?cat=1 ,https://wp-themes.com/blocksy/?cat=1,

Here is 2.x version : https://wp-themes.com/neve/?cat=1 (it has separation)

@selul
Copy link
Contributor

selul commented Jul 26, 2021 via email

@ineagu
Copy link
Author

ineagu commented Jul 26, 2021

What has to do excerpt from initial comment with this ?

sorry missed that part. If you look in the screenshot, you would see that we display the featured image twice( I am not sure if it's the featured image or we grab the first image from the content)

@JohnPixle
Copy link

I think there is something special with the Code in WP single posts, that contributes to making the category archive page looking a bit confusing, but I cannot properly evaluate it from the frontend. I think the featured image appears in the content as well?

@abaicus
Copy link
Collaborator

abaicus commented Jul 26, 2021

  1. Categories aren't looking great, maybe we can tweak the defaults.

I am not sure in what environment you are testing, what plugins are active, etc. You can see how the category archive acts here.

I think there is something special with the Code in WP single posts, that contributes to making the category archive page looking a bit confusing, but I cannot properly evaluate it from the frontend. I think the featured image appears in the content as well?

Probably @JohnPixle is right about this.


  1. Neither Astra nor Kadence display the category descriptions within the menu like we do

We can remove that, but as far as I remember users requested this at the beginning. It's the actual menu item description and we only show it if it's defined per menu item. If you define it I'm guessing you'd like it to show up if you explicitly set it. Should we remove it?


  1. I would say that pages should not have the sidebar enabled by default.

We can either default to the advanced settings on the new skin and have no sidebar for pages from the customizer defaults, or simply use a default theme data that has no sidebar (but that won't work for non-fresh sites).

@ineagu
Copy link
Author

ineagu commented Jul 26, 2021

  1. Categories aren't looking great, maybe we can tweak the defaults.

I am not sure in what environment you are testing, what plugins are active, etc. You can see how the category archive acts here.

I think there is something special with the Code in WP single posts, that contributes to making the category archive page looking a bit confusing, but I cannot properly evaluate it from the frontend. I think the featured image appears in the content as well?

Probably @JohnPixle is right about this.

  1. Neither Astra nor Kadence display the category descriptions within the menu like we do

We can remove that, but as far as I remember users requested this at the beginning. It's the actual menu item description and we only show it if it's defined per menu item. If you define it I'm guessing you'd like it to show up if you explicitly set it. Should we remove it?

  1. I would say that pages should not have the sidebar enabled by default.

We can either default to the advanced settings on the new skin and have no sidebar for pages from the customizer defaults, or simply use a default theme data that has no sidebar (but that won't work for non-fresh sites).

  1. ok, cleared.

  2. If is the menu item description, I think we should keep it. However that is the category description, which I don't think we should display.

@abaicus
Copy link
Collaborator

abaicus commented Jul 26, 2021

  1. If is the menu item description, I think we should keep it. However that is the category description, which I don't think we should display.

It is not the category description that shows up as I tested it before writing my last reply. 😄
That menu item must have a description itself.

@ineagu
Copy link
Author

ineagu commented Jul 26, 2021

I did tested it as well just before replying :D

It seems that WordPress by default is copying the category description as the menu description when you add categories in the menu :D which is confusing, because the field is hidden by default. I guess is safer to hide by default and enable with a hook/doc/setting.

@abaicus
Copy link
Collaborator

abaicus commented Jul 26, 2021

It seems this only happens if you add the category description before you add the item. I'm not sure what we should do in that case as the description field fills itself.

I don't know what we should do here. Should we simply hide it when we're using the new skin and add a filter to display it?

@selul
Copy link
Contributor

selul commented Jul 26, 2021

Ok to summarize this, let's agree on doing the next things:

  • Review the font sizes, @JohnPixle will come up with the final values today and sync with @abaicus to have them into the theme.
  • For meta spacing: For now let’s just fix the default spacing. By default, there is a 30px bottom margin applied to the H1 and H2 which cascades to the post titles in single and archive. This is what makes the spacing so big there. We can use half of this value (15px) applied specifically to title
  • Can we switch the default archive style to 3x3 as @JohnPixle outlined here
    image without the sidebar?
  • We can keep the current menu description behavior, no special treatment for now.
  • Let's make sure the pages have no sidebar by default, if is easier we can make it available only on the new skin.

That's all, thank you for sharing your thoughts.

@abaicus
Copy link
Collaborator

abaicus commented Jul 26, 2021

@selul @JohnPixle I combed through the discussion again and wanted to add this to the list of things that need to be done:

  • Add a margin to li tags

image

This was referenced Jul 26, 2021
@selul selul added the bug This label could be used to identify issues that are caused by a defect in the product. label Jul 27, 2021
@pirate-bot
Copy link
Collaborator

🎉 This issue has been resolved in version 3.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@pirate-bot pirate-bot added the released Indicate that an issue has been resolved and released in a particular version of the product. label Aug 4, 2021
@selul selul closed this as completed Aug 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This label could be used to identify issues that are caused by a defect in the product. released Indicate that an issue has been resolved and released in a particular version of the product.
Projects
None yet
Development

No branches or pull requests

6 participants