#863 Styles for pictures

Merged
max.mehl merged 7 commits from feature/828/picture-style into master 4 months ago

Here is my draft for the picture styles. See the images attached for examples.

@max.mehl @eal what do you think?

Closes #828

PS: I also experimented with some borders (PMPC images). Just for fun :)

Here is my draft for the picture styles. See the images attached for examples. @max.mehl @eal what do you think? Closes #828 PS: I also experimented with some borders (PMPC images). Just for fun :)
mweimann commented 5 months ago
Poster

Updated picture doc

Updated picture doc
max.mehl was assigned by mweimann 5 months ago
eal was assigned by mweimann 5 months ago
mweimann added the
design
label 5 months ago
reinhard commented 5 months ago
Collaborator

Even though I was not asked, I hope you don’t mind if I add two questions :-)

  1. Would it be possible to achieve the same design without the need for an encapsulating <div> around the <figure>?

  2. Wouldn’t we want to define some reasonable defaults for the case when <figure> and <figcaption> are used without any specific classes?

Both would, IMHO, make the HTML code required to display an image much more readable and understandable.

Even though I was not asked, I hope you don't mind if I add two questions :-) 1. Would it be possible to achieve the same design without the need for an encapsulating `<div>` around the `<figure>`? 2. Wouldn't we want to define some reasonable defaults for the case when `<figure>` and `<figcaption>` are used without any specific classes? Both would, IMHO, make the HTML code required to display an image much more readable and understandable.
mweimann commented 5 months ago
Poster

Even though I was not asked, I hope you don’t mind if I add two questions :-)

I’m happy about any feedback!

Would it be possible to achieve the same design without the need for an encapsulating

around the
?

This is required for centering and spacing. If you don’t use the wrapper it’s left aligned without spacing. For floating images you also don’t need the wrapper (see example).

Wouldn’t we want to define some reasonable defaults for the case when

and
are used without any specific classes?

It’s possible, but then you can’t have figures easy in another way. Let’s wait for other feedback about this point.

> Even though I was not asked, I hope you don’t mind if I add two questions :-) I'm happy about any feedback! > Would it be possible to achieve the same design without the need for an encapsulating <div> around the <figure>? This is required for centering and spacing. If you don't use the wrapper it's left aligned without spacing. For floating images you also don't need the wrapper (see example). > Wouldn’t we want to define some reasonable defaults for the case when <figure> and <figcaption> are used without any specific classes? It's possible, but then you can't have figures easy in another way. Let's wait for other feedback about this point.
reinhard commented 5 months ago
Collaborator

Thank you @mweimann for your quick response!

For me, one of the goals of a global CSS is to ensure a consistent look and feel across the website rather than to allow a variety of different designs; the CSS is not only a toolbox to pick possible classes from, it is also our definition of the default style of elements. I am not sure though how much others share this view.

For me, an ideal solution would be that if I write…

<figure>
  <img source="..." alt="..."/>`
  <figcaption>Some nice text</figcaption>
</figure>

…in our HTML code (without encapsulating <div>), the image appears exactly the way how we want captioned images to appear on our web pages, except for special cases where we add an explicit class (like to right-align it and float text around it).

Thank you @mweimann for your quick response! For me, one of the goals of a global CSS is to ensure a consistent look and feel across the website rather than to allow a variety of different designs; the CSS is not only a toolbox to pick possible classes from, it is also our definition of the default style of elements. I am not sure though how much others share this view. For me, an ideal solution would be that if I write... ```html <figure> <img source="..." alt="..."/>` <figcaption>Some nice text</figcaption> </figure> ``` ...in our HTML code (without encapsulating `<div>`), the image appears exactly the way how we want captioned images to appear on our web pages, except for special cases where we add an explicit class (like to right-align it and float text around it).
eal commented 5 months ago
Collaborator

I am on Reinhard’s page here. Embedding pictures is elementary to all pages and a “daily operation”. It should be as easy as possible to embed pictures in fsfe-style without having to open another web-page, look at the source-code and copy-paste it (what I actually currently do). Looking at https://git.fsfe.org/attachments/9a45dbe7-22d7-4127-a111-5c9d7843ad98 there are so many classes used that it is hard to memorize and easy to make an error. A solution that offers:

figure “class=fullpic” (or this should actually happen without a class)

figure “class=picleft” figure “class=picright”

figcaption “class=textcenter”

and no more classes, instead, would be a big help to code by heart.

and btw: how can I embed snippets here in the comment like you do?

I am on Reinhard's page here. Embedding pictures is elementary to all pages and a "daily operation". It should be as easy as possible to embed pictures in fsfe-style without having to open another web-page, look at the source-code and copy-paste it (what I actually currently do). Looking at https://git.fsfe.org/attachments/9a45dbe7-22d7-4127-a111-5c9d7843ad98 there are so many classes used that it is hard to memorize and easy to make an error. A solution that offers: figure "class=fullpic" (or this should actually happen without a class) figure "class=picleft" figure "class=picright" figcaption "class=textcenter" and no more classes, instead, would be a big help to code by heart. and btw: how can I embed snippets here in the comment like you do?
reinhard commented 5 months ago
Collaborator

I suggest <figure class="pull-left"> and <figure class="pull-right"> instead of picleft and picright for consistency with the standard bootstrap classes. And I would go even further and say that the figcaption should be centered below the figure by default.

@eal: to embed a code snippet within a line, surround it with single backticks, and to embed a multi-line code snippet, surround it with triple backticks: https://github.com/adam-p/markdown-here/wiki/Markdown-Cheatsheet#code

I suggest `<figure class="pull-left">` and `<figure class="pull-right">` instead of picleft and picright for consistency with the standard bootstrap classes. And I would go even further and say that the figcaption should be centered below the figure by default. @eal: to embed a code snippet within a line, surround it with single backticks, and to embed a multi-line code snippet, surround it with triple backticks: https://github.com/adam-p/markdown-here/wiki/Markdown-Cheatsheet#code
eal commented 5 months ago
Collaborator

backticks is weired, but thanks for the hint of how to embed <span>snippets</span>

pull-left and pull-right would be ok with me (i really do not care about the name of the class), but my idea was to have a class picright that already combines multiple classes from bootstrap which is for examle pull-left, width: 50%, padding: 1em, shadow (please note this is just an example no real proposal of code, I know it#s not proper). Such a super-class would make it a way easier for everyone who does not know bootstrap. But of course such a super-class cannot have the name of a default class from bootstrap.

About the figcaption: I think it depends. One line caption should usually be centered. If you have two or more lines in a caption, however, centered might be unpleasant to read.

backticks is weired, but thanks for the hint of how to embed ``<span>snippets</span>`` ``pull-left`` and ``pull-right`` would be ok with me (i really do not care about the name of the class), but my idea was to have a class ``picright`` that already combines multiple classes from bootstrap which is for examle ``pull-left, width: 50%, padding: 1em, shadow`` (please note this is just an example no real proposal of code, I know it#s not proper). Such a super-class would make it a way easier for everyone who does not know bootstrap. But of course such a super-class cannot have the name of a default class from bootstrap. About the figcaption: I think it depends. One line caption should usually be centered. If you have two or more lines in a caption, however, centered might be unpleasant to read.
reinhard commented 5 months ago
Collaborator

With CSS you can easily define that, for example, if the pull-left class is applied to a <figure>, it defaults to a width of 25% and a padding of 1em; shadowed style should (IMHO) be the default for <figure> anyway.

About the figcaption: making centered text the default can still leave us the option open to add a text-left or text-right class, it’s just a question of what we want to make the default.

With CSS you can easily define that, for example, if the pull-left class is applied to a `<figure>`, it defaults to a width of 25% and a padding of 1em; shadowed style should (IMHO) be the default for `<figure>` anyway. About the figcaption: making centered text the default can still leave us the option open to add a text-left or text-right class, it's just a question of what we want to make the default.
eal commented 5 months ago
Collaborator

Ok, I agree: per default we define figure to be full-size, centered with space and figcaption to be centered. We define nice classes for pictures to pull-left or pull-right and i necessary have text-left or text-right to fine-grain.

Ok, I agree: per default we define ``figure`` to be full-size, centered with space and ``figcaption`` to be centered. We define nice classes for pictures to ``pull-left`` or ``pull-right`` and i necessary have ``text-left`` or ``text-right`` to fine-grain.
mweimann commented 5 months ago
Poster

Thanks for the feedback.

See the screenshot of the template page for a new style version.

PS: My proposal wasn’t intended to be complicated, instead I tried to match the bootstrap way of doing it (kind of):
https://getbootstrap.com/docs/4.3/content/figures/

Thanks for the feedback. See the screenshot of the template page for a new style version. PS: My proposal wasn't intended to be complicated, instead I tried to match the bootstrap way of doing it (kind of): https://getbootstrap.com/docs/4.3/content/figures/
reinhard commented 5 months ago
Collaborator

Awesome! I think this is great!

Only some minor nitpicks, please feel free to consider or ignore as you see fit :-)

  • The text on the template page still speaks of the (now gone) div wrapper.
  • #eee could be replaced with @gray-lighter, which would be more bootstrap-ish.
  • Likewise, @font-size-small instead of .9em
  • Likewise, @screen-md-min instead of 992px

Other than that, I think it’s perfect, thank you so much for providing this!

Awesome! I think this is great! Only some minor nitpicks, please feel free to consider or ignore as you see fit :-) * The text on the template page still speaks of the (now gone) div wrapper. * `#eee` could be replaced with `@gray-lighter`, which would be more bootstrap-ish. * Likewise, `@font-size-small` instead of `.9em` * Likewise, `@screen-md-min` instead of `992px` Other than that, I think it's perfect, thank you so much for providing this!
mweimann commented 5 months ago
Poster

✓ done

#eee could be replaced with @gray-lighter, which would be more bootstrap-ish. Likewise, @font-size-small instead of .9em

Cool - didn’t know there is still some working bootstrap around :)

Likewise, @font-size-small instead of .9em

I decided against, because then the text looks too small.

✓ done > \#eee could be replaced with @gray-lighter, which would be more bootstrap-ish. Likewise, @font-size-small instead of .9em Cool - didn't know there is still some working bootstrap around :) > Likewise, @font-size-small instead of .9em I decided against, because then the text looks too small.
mweimann changed title from WIP: Styles for pictures to Styles for pictures 5 months ago
mweimann commented 5 months ago
Poster

If this is fine now, should we merge this one @max.mehl

If this is fine now, should we merge this one @max.mehl
max.mehl commented 4 months ago
Owner

Sorry, I didn’t notice I was the blocker here!

Thanks for the work @mweimann. @eal and @reinhard, I am merging this now so you can start using the awesome new styles :)

Sorry, I didn't notice I was the blocker here! Thanks for the work @mweimann. @eal and @reinhard, I am merging this now so you can start using the awesome new styles :)
reinhard referenced this issue from a commit 4 months ago
The pull request has been merged.
Sign in to join this conversation.
Loading…
Cancel
Save
There is no content yet.