Adding two new templates for more flexibility

paulcmal commented 8 years ago

aboutme_sidebar.html.twig is designed to have the profile pic, name and social media links in the sidebar. aboutme_inline.html.twig is an configurably-visible div containing the profile pic, name and URL, ideal for embedding authorship info within blog posts

I'm currently using this sidebar.html.twig and this blog_item.html.twig.

I added two new config options :

one for displaying/hiding aboutme info in the blog items themselves : defaults to false (so that it doesn't add too much visual information, yet allows mf2-compliant websites to parse it)
one for enabling/disabling aboutme in the sidebar : defaults to true

The sidebar might benefit some improvement but I think the overall design could be worse :)

You can see the result here.

paulcmal commented 8 years ago

I think I'm about done here.

If you agree to merge this PR, I'll gladly submit a PR to get my changes to sidebar.html.twig blog_item.html.twig merged into antimatter so that people installing this plugin can see it in action without modifying their default templates.

See the diff here.

seb-vial commented 8 years ago

Thanks for the effort, but I'm not sure it's such a good idea to change the name of the template rendered in the sidebar... Antimatter is not the only existing theme... Even though the plugin is probably not used by too many people, every person who's not using antimatter or using his/her own theme would have to change their template too.

About the inline template, that would mean that every blog post is written by the author of the blog, but I'm sure there are many blogs with many authors, so of course you can hide it, but the information is still there.

paulcmal commented 8 years ago

to change the name of the template rendered in the sidebar

Well, it's just a new template with minimal information. I happened to use it in the sidebar (which is why I made it so short in the first place). It may as well be renamed to something else like aboutme_social.html.twig?

every person (…) would have to change their template too.

Correct me if I'm wrong, but I didn't remove your original template so I don't think there's going to be any "mandatory" change.

that would mean that every blog post is written by the author of the blog

As a temporary workaround, yes, because that's Grav's default assumption at the moment. But once there is proper authorship markups in Grav pages, I'll be glad to implement multi-user support!

I tried not to add any breaking changes to be able to merge upstream, but if you'd prefer me to maintain a fork, that's also okay. But that's more efforts in the long run, so if you have any pointers on what I should change to get this merged, I'm listening.

I'd propose:

renaming templates/partials/aboutme_sidebar.html.twig to aboutme_social.html.twig
removing the the "Display in sidebar" option and let users manually edit their templates (as recommended in the README
edit the README to make it more clear that the "aboutme_social.html.twig" template can be used just about anywhere and not just in the sidebar
keeping the display/hide inline authorship in items

What do you think?

seb-vial commented 8 years ago

Well, it's just a new template with minimal information. I happened to use it in the sidebar (which is why I made it so short in the first place). It may as well be renamed to something else like aboutme_social.html.twig?

The name is not very important I guess.

Correct me if I'm wrong, but I didn't remove your original template so I don't think there's going to be any "mandatory" change.

I thought you wanted to replace the only template by two actually... My bad, but the aboutme_sidebar.html.twig template would be a duplicate of the original aboutme.html.twig template.

Plus you changed the structure of the template, adding a h4 title with Social Media (that would require translation), and, the name is now next to the profile picture followed by is also available on… (translation needed here too). It means the sidebar would be larger that it needs to be, I would not recommend this. I think the author's name needs to be bigger than the description. Also Social Media would not be the term I would use, the plugin is not only about author's social pages.

At first I made it so there was no need for translation. I'm not against adding some more information (title, etc) but it would need translations, like I did with my theme.

As a temporary workaround, yes, because that's Grav's default assumption at the moment. But once there is proper authorship markups in Grav pages, I'll be glad to implement multi-user support!

Perhaps you're right, but Grav already permits having different users being able to write pages/blog posts. Maybe I should think of a way to manage these users in the plugin and then we could add your second template, in the meantime I'd rather not if that's ok with you. Or another solution would be to use the option you made to hide the inline template or not. Instead of adding display: none you should simply not insert the template at all.

paulcmal commented 8 years ago

Plus you changed the structure of the template, adding a h4 title with Social Media (…) and, the name is now next to the profile picture followed by is also available on…

I'm not against removing the <h4>.

It means the sidebar would be larger that it needs to be

Actually not. I'm currently working on a 1024x768 setup and here is what I get displayed. Fitting everything in nicely is precisely the reason I did not use the profile name as <h4>.

I think the author's name needs to be bigger than the description.

That's a fair point.

Also Social Media would not be the term I would use, the plugin is not only about author's social pages.

True, but are you against having a template tailored to this use-case?

I'm not against adding some more information (title, etc) but it would need translations, like I did with my theme.

I couldn't agree more. I was planning on doing this later. But that's a good point to get translations going before merging.

Maybe I should think of a way to manage these users

I have a few ideas regarding this, but I'd propose to open a separate ticket to talk about it.

Instead of adding display: none you should simply not insert the template at all.

How about both options? Enable/Disable and Show/Hide. EDIT: Actually, I don't get it. Users currently need to manually add it to their template for it to be even enabled, so isn't a enable/disable option a bit redundant? Sure, if my templates modifications get merged to antimatter upstream someday that would be useful, but for the moment I don't really see the point. I'll still implement it.

So in addition to my previous proposals, I'd add:

removing the <h4>Social media</h4> from the "social" template
adding translation strings for "is also available on…"
adding enabling/disabling item authorship

I'll close this PR until my commits are ready. Would you agree to merge when I get all these changes? Or do you have any additional requests/suggestions?

seb-vial commented 8 years ago

Actually not. I'm currently working on a 1024x768 setup and here is what I get displayed. Fitting everything in nicely is precisely the reason I did not use the profile name as
.

Yes but most people are using 1920x1080, which means that the author's name and is also available on are next to the picture, it's kinda ugly to me... Also, is it really necessary to say is also available on ? When you display social media icons, people are already aware the author is also available on these social networks.

How about both options? Enable/Disable and Show/Hide

I agree

If we go with this, and your PR for the antimatter theme is accepted, the default template aboutme.html.twig would not be needed anymore except for compatibility with themes which are still using this template.

paulcmal commented 8 years ago

Yes but most people are using 1920x1080, which means that the author's name and is also available on are next to the picture, it's kinda ugly to me...

Hmm, I didn't see this coming. I'll get on this.

Also, is it really necessary to say is also available on ?

No, it's not. I just found it was a convenient way to make things clear to people browsing the page. If none of us has strong argument for either keeping or removing it, I may as well make a configurable option to enable/disable it. What do you say?

I'm also tempted to add a show/hide option to this template so that people may actually use it for parsing/bots/indieweb purposes without having to display it (pretty much like for the inline template).

Should I just add enable/disable and show/hide for this template as well?

seb-vial commented 8 years ago

Hmm, I didn't see this coming. I'll get on this.

That's why I used h4, as the author's name should be bigger and in term of semantics must also be more important.

No, it's not. I just found it was a convenient way to make things clear to people browsing the page. If none of us has strong argument for either keeping or removing it, I may as well make a configurable option to enable/disable it. What do you say?

Indeed it also could be an option.

Also I saw that you added an option to show/hide the social media icons as well ?

EDIT: In your new template, where's the description btw... ?

paulcmal commented 8 years ago

Also I saw that you added an option to show/hide the social media icons as well ?

That was in the standard template. It's only there to prevent <div class="social-pages"> from appearing if config.plugins.aboutme.social_pages.enabled is false.

seb-vial commented 8 years ago

Ah, forgot about this ^^ But no description in your template...

paulcmal commented 8 years ago

But no description in your template...

Currently working on it!

Also, I guess since there's already config.plugins.aboutme.social_pages.enabled we could just use this as a condition to enable/disable aboutme_social.html.twig as a whole and then just add one parameter to display/hide it.

What do you think?

seb-vial commented 8 years ago

What do you mean ? If aboutme_social is supposed to be used in the sidebar (instead of aboutme), you can simply let the {% if config.plugins.aboutme.enabled %} condition in the sidebar template ?

paulcmal commented 8 years ago

What do you mean ? If aboutme_social is supposed to be used in the sidebar (instead of aboutme), you can simply let the {% if config.plugins.aboutme.enabled %} condition in the sidebar template ?

Yes but some people might want to disable this specific template to use another one. Or maybe I'm overthinking it? I guess it's pretty straightforward with all the options available in the config.

I still need to fix a little nasty bug but here's where I am on my todo-list:

[x] renamed aboutme_sidebar.html.twig to aboutme_social.html.twig
- [x] removed the "Display in sidebar" option
[x] translation strings
[x] better display for social template
- [x] centered everything in the template
- [x] removed the <h4>
- [x] fix problem of spamming a <div class="sidebar-content"> (and useless spacing) when social pages enabled but not displayed
[x] config options
- [x] enable_item_authorship (defaults to false)
- [x] display_item_authorship (defaults to false)
- [x] social_pages.display (defaults to true)
- [x] social_pages.show_text (defaults to false)
[x] updated the README to reflect all this

For this useless div issue, I had to create a aboutme_sidebar.html.twig template as a wrapper around aboutme_social.html.twig so that one could write in their sidebar.html.twig:

{% if config.plugins.aboutme.enabled %}
  {% include 'templates/partials/aboutme_sidebar.html.twig' %}
{% endif %}

See here for the latest version. It's all online here.

Anything else you'd like me to add/change/remove or can I reopen the PR?

seb-vial commented 8 years ago

So it means that when the user decides not to display social buttons, the whole plugin is not displayed even though they have their name, profile picture, title and description ?

{% if config.plugins.aboutme.social_pages.enabled %}
<div class="sidebar-content" {% if not config.plugins.aboutme.social_pages.display %}style="display: none;"{% endif %}>
    {% include 'partials/aboutme_social.html.twig' %}
</div>
{% endif %}

It does not really make sense, when users decide to enable the aboutme plugin, they also have to enable social buttons so the plugin shows, so either config.plugins.aboutme.social_pages.enabled is useless or misused ?

And btw, the title and the description are still missing from your template.

paulcmal commented 8 years ago

So it means that when the user decides not to display social buttons, the whole plugin is not displayed even though they have their name, profile picture, title and description ?

No, just this specific template. I took this decision trying to cover the use-case of someone using a theme embedding by default partials/aboutme_social.html.twig, but who does not wish to use it (say, to place it somewhere else).

But upon second thought, I think you're right about this. I will edit it to add the template in the sidebar if config.plugins.aboutme.enabled as before. Or introducing a new option like aboutme.show_in_sidebar.

Which options fits you best?

And btw, the title and the description are still missing from your template.

That was intentional in this specific template, as it takes way too much space (especially with the default description, maybe we could change it?). Try this page in 1024x768. I don't think it's possible to display the description properly.

There's many workarounds for this, like for instance adding settings to show/hide the different elements in aboutme_social.html.twig (as well as providing a shorter description by default).

I think these issues would be way easier to discuss using instant messaging. We could probably come up with a decent solution if we talked 10 minutes about it. Would you have time to talk on Gitter, maybe?

seb-vial / grav-plugin-about-me

Adding two new templates for more flexibility #5

.