pmoreno-rodriguez / grav-theme-future2021

Future Imperfect theme by HTML5UP ported from scratch to Grav. Version 2021
Other
20 stars 10 forks source link

Merge changes and fixes into develop #25

Closed pikim closed 1 year ago

pikim commented 1 year ago

Why did you do https://github.com/pmoreno-rodriguez/grav-theme-future2021/pull/25/commits/36d4668000505d59723475c65449ade7f9fd83c1? I replaced a hr tag with it in https://github.com/pikim/grav-theme-future2021/commit/7909c59e83799975af55379bc1c63c23f2717bf0 because all other lines and separators are also done in css.

pikim commented 1 year ago

All the other things look ok to me.

pmoreno-rodriguez commented 1 year ago

Why did you do 36d4668? I replaced a hr tag with it in pikim@7909c59 because all other lines and separators are also done in css.

I think with border to none, it's more clean. With border defined in css, in my opinion there are too much lines. See the example below:

Search results with border:

search-row

Search resultos wihout border:

search-row2

pikim commented 1 year ago

I think with border to none, it's more clean. With border defined in css, in my opinion there are too much lines. See the example below:

In my opinion this is because you didn't remove the <hr> tag in https://github.com/pmoreno-rodriguez/grav-theme-future2021/blob/develop/templates/partials/simplesearch_item.html.twig#L22

Interestingly, I have no lines with your latest develop, see https://github.com/pmoreno-rodriguez/grav-theme-future2021/issues/26#issuecomment-1416843592 whereas I have a single line for each entry with my latest develop.

pikim commented 1 year ago

@pmoreno-rodriguez I merged your latest changes into my develop branch. I have one single line per search item now, but result pages containing tables still get messed up. The same happens with pictures. So the result still looks like https://github.com/pmoreno-rodriguez/grav-theme-future2021/issues/26#issuecomment-1416843592 but with separation lines now.

Maybe you can test all the changes together by checking out my develop branch. It contains all your changes and all my changes at the moment. That means it has the same state that your develop would have if you accepted this pull request.

pikim commented 1 year ago

@pmoreno-rodriguez All your commits you referred to here look OK to me. So from my point of view this PR can be merged.

pmoreno-rodriguez commented 1 year ago

@pmoreno-rodriguez I merged your latest changes into my develop branch. I have one single line per search item now, but result pages containing tables still get messed up. The same happens with pictures. So the result still looks like #26 (comment) but with separation lines now.

Maybe you can test all the changes together by checking out my develop branch. It contains all your changes and all my changes at the moment. That means it has the same state that your develop would have if you accepted this pull request.

@pikim , I don't understand what you want to say with "...but result pages containing tables still get messed up...". Does this mean that if there are tables on a page, the summary on the search results page is messed up?

pikim commented 1 year ago

@pikim , I don't understand what you want to say with "...but result pages containing tables still get messed up...". Does this mean that if there are tables on a page, the summary on the search results page is messed up?

Yes, exactly. See: grafik

pmoreno-rodriguez commented 1 year ago

Have you tried the |safe_truncate_html twig filter in summary?

pikim commented 1 year ago

Like this: <p>{{ page.summary|truncate(200)|safe_truncate_html }}</p>? Then I don't see the picture any more and the other sites aren't messed up. But instead I get something like

<h2>Willkommen auf unseren Seiten</h2> <p><img alt="" src="/user/pages/startseite/vereinsfoto_2017.jpg" /></p> <h3>Schießzeiten</h3> <p>Jugend: Freitag von 18:00 bis 20:00<br /> Erwachsene: Montag und&hellip;

or

<table> <thead> <tr> <th style="text-align: left;">Disziplin</th> <th style="text-align: left;">2003</th> <th style="text-align: left;">2004</th> </tr> </thead> <tbody> <tr> <td style="text-align: lef&hellip;

as result text.

pmoreno-rodriguez commented 1 year ago

@pikim , I don't understand what you want to say with "...but result pages containing tables still get messed up...". Does this mean that if there are tables on a page, the summary on the search results page is messed up?

Yes, exactly. See: grafik

@pikim I'd like to see this on your demo site, but I can't see the thumbnails or summary in the search results. Did you change the code? Could you email me some pages of the (zipped) content from your demo site, to create my own fork?

pmoreno-rodriguez commented 1 year ago

Hi @pikim. I think I have a possible solution for search results with tables.

Try changing {{ page.summary|truncate(200, true)|raw }}, adding true to the truncate filter and removing the <p> tag (to avoid an extra paragraph). On the other hand, I've changed the misc.css to add again width and height to the search item img, while removing the cropZoom filter from simplesearch_item.html.twig. I did this to improve svg support in simple search results.

pikim commented 1 year ago

I'd like to see this on your demo site

You can see it now by adding /search/query:jugend to the URL you already know.

Try changing {{ page.summary|truncate(200, true)|raw }}, adding true to the truncate filter and removing the <p> tag (to avoid an extra paragraph).

This doesn't help unfortunately. It looks even worse as it also messes up the layout.

pmoreno-rodriguez commented 1 year ago

Hi @pikim. Finally I think I found a solution. Try to set this to remove images and tables from summary:

{{ page.summary|striptags('img table')|truncate(300, true)|raw }}

Note: See help in Discourse .

pmoreno-rodriguez commented 1 year ago

With {{ page.summary|striptags|truncate(300, true)|raw }} also works fine.

pikim commented 1 year ago

With {{ page.summary|striptags|truncate(300, true)|raw }} also works fine.

Unfortunately not completely. There is a problem with <div id="main"> and the item separators on the search results page now. You can see this on my current test site using the search term from above.

pmoreno-rodriguez commented 1 year ago

I only see problems with the separator bar on pages with an event template. However, this is a minor problem, which we will be able to improve in later versions. I think it's a good time to release the new version and improve these little bugs in the future. I would like you to try the new mods, before releasing the new version. Definitely, I have decided that the option to show sidebar is defined in each page, to give more flexibility to the user to choose which pages show or not the sidebar. Likewise, I have left the sidebar variable of the theme, for those pages in which the user does not create a page, and therefore does not choose if it is displayed or not, but rather are pages that are used for other purposes (eg, offline page , simplesearch results, etc.).

I look forward to your contributions.

pmoreno-rodriguez commented 1 year ago

I have no problem with pages without any image. The bottom border takes up all the space.

Screenshot from 2023-02-06 21-21-32

pikim commented 1 year ago

I have no problem with pages without any image. The bottom border takes up all the space.

Regarding the border/separator: I have some more CSS for the search-items, maybe it's interesting for you:

.search-item {
  margin-bottom: 0;
}

.search-row {
  padding-bottom: 1.25em;
  margin-bottom: 1.5em;
}

.search-row:last-child {
  border: 0;
  padding-bottom: 0;
}

.search-row a {
  border: 0;
}

Regarding <div id="main">: I would have expected that the search results are placed inside a white box as they always were before. This is not the case with the latest version any more. If this is intentional forget my writings about that topic. But to be honest I preferred the style were it's inside the white box.

You can compare both topics on my productive site (custom CSS and with white box) and my test site (my develop, which is just some commits older than your develop).

By the way: there's a duplicate line near the menu on your demo site. I didn't find out where it comes, yet. grafik

pmoreno-rodriguez commented 1 year ago

@pikim. I have done some changes in theme today. Regarding to duplicated line in menu (if no language is set in system settings) is already fixed in develop branch.

Regarding the search results, I've been doing some modifications to the css files, and if I set <article class="simplesearch"> to <article class="post"> The Results appear with a white background, but it inherits some classes from the post style and the thumbnails don't look right (they are cropped, not with width and height to 100px )

Could you help me to improve this issue?

See this screenshot:

Screenshot from 2023-02-07 19-52-08

pmoreno-rodriguez commented 1 year ago

Try the last changes, uploaded to develop branch. Finally I've could to modify the css styles for simplesearch in misc.css. I think it look right.

pikim commented 1 year ago

It looks good and it's great that you were able to fix the issues! Great work!!! I'm looking forward to the next release.

pmoreno-rodriguez commented 1 year ago

Hi @pikim.

It has been two days since version 1.0.3 of the Future2021 theme was released. I hope you enjoy it and I hope to continue counting on your collaboration.

I am currently doing some modification in the blog_item and miniposts files, so that they choose the first image of the page if this is not the avatar image, with support for all types of images, including SVG. They are already in the development branch.

Check out this Discourse post