Replace hero image with carousel slider #544

TimOsahenru · 2024-10-26T21:14:56Z

Issue Link 🔗:

Issue: #365

Type of Change

Bug fix 🐞
New feature/page
Documentation update
Other

Description 📋

Replaced static hero image with a dynamic Bootstrap carousel for improved user experiece.

Checklist ✅

Followed the Code of Conduct and Contribution Guide
Ran pre-commit run --all
All tests pass locally
Added tests (if applicable)
Documentation updated (if applicable)

Additional Notes & Screenshots

Add any additional notes or comments that might be helpful for the reviewers.

kjaymiller · 2024-10-28T13:18:53Z

@TimOsahenru I'm glad to hear that you were able to get the tests passing? What did you wind up doing in the end?

kjaymiller · 2024-10-28T13:53:18Z

@TimOsahenru,

This is a great start!

There are a couple of things that I would like to see refined before we go live with the changes. That said I don't want to lose this work so I've created a new branch called New-Carousel... can you submit the PR to that branch and then we can look at some of the things that I would hope that we can get touched up?

Quick List of things to address:

Only show the blog posts that are in recent posts. This also means that we can de-emphasize that section and make it smaller.
The circle image makes it hard to see some images, it may be better as a full width image`
With the new featured image setup, we should be able to use that for articles with
The descriptions seem like the first sentence of the content and not a dedicated line that can be used for MetaTagging as well. - I recently created a tool that would generate autodescriptions for my personal blog that could be re-used for this. - https://github.com/kjaymiller/kjaymiller.com/blob/main/ollama_summarizer.py `
The section is margined inside the container making it smaller than the content near it. We can fix that by changing the width of the container from 85% to 100%
The default image seems to be a little buggy. Instead of giving the default image it like creates an image of the raw content... we can fix this by removing default: post.content | split: '!' | first | split: '(' | last | split: ')' | first |

kjaymiller

@TimOsahenru great work on this...

I know there are a lot of comments which is why I want to use this as the start for the work in New Carousel and then iterate on that work to get it live on the page. This has been excellent so far!

kjaymiller · 2024-10-28T13:19:32Z

tests/test.py

@@ -12,7 +12,7 @@ def page_url(xprocess, url_port):
    url, port = url_port

    class Starter(ProcessStarter):
-        timeout = 4
+        timeout = 20


Ah so your server wasn't ready yet. This is a good solution... great work troubleshooting and catching this!!

kjaymiller · 2024-10-28T13:55:41Z

_layouts/default.html

@@ -12,6 +12,7 @@
    <link rel="canonical" href="{{ page.url | replace:'index.html','' | prepend: site.baseurl | prepend: site.url }}" />
    <link rel="alternate" type="application/rss+xml" title="{{ site.title }}" href="{{ '/feed.xml' | prepend: site.baseurl | prepend: site.url }}" />
    <link rel="icon" href="https://fav.farm/%E2%9C%8A%F0%9F%8F%BE" type="image/svg" />
+    <link href="https://cdn.jsdelivr.net/npm/[email protected]/dist/css/bootstrap.min.css" rel="stylesheet">


You chose to use bootstrap. Interesting... I wonder what the load time difference is between using bootstrap versus a solution that uses css with transitions and animations.

Adding bootstrap for a singular component doesn't feel great.

You’re absolutely right; I hadn’t fully considered the impact of load time when choosing Bootstrap. I felt custom CSS might be re-inventing the wheel.

I’ll look into using a custom CSS and explore the impact both has on our load time. Thanks for the insight

kjaymiller · 2024-10-28T13:56:25Z

_layouts/index.html

+            <img src="{{ post.featured_image | default: post.content | split: '!' | first | split: '(' | last | split: ')' | first | default: 'default-image.jpg' }}" alt="{{ post.title }}">
+          </div>
+          <h3 class="post-title">{{ post.title }}</h3>
+          <p class="post-summary">{{ post.excerpt | strip_html | truncatewords: 20 }}</p>


This creates some interesting issues... Perhaps we can request a description on each article (I mentioned this in the comments)

kjaymiller · 2024-10-28T13:57:08Z

_layouts/index.html

+        {% for post in site.posts %}
+        <div class="carousel-item {% if forloop.first %}active{% endif %}">
+          <div class="email-circle">
+            <img src="{{ post.featured_image | default: post.content | split: '!' | first | split: '(' | last | split: ')' | first | default: 'default-image.jpg' }}" alt="{{ post.title }}">


This is creating some issues in that the default image is not being selected.

Gotcha! Working on it. Thanks

kjaymiller · 2024-10-28T13:58:03Z

assets/css/bpd.css

+  padding: 40px;
+  border-radius: 10px;
+  margin: 0 auto;
+  width: 85%;


This width inside the container makes the carousel smaller than everything around it. I'd suggest making it 100%.

kjaymiller · 2024-10-28T13:58:46Z

assets/css/bpd.css

+  width: 85%;
+  /* max-width: 1000px; */
+}
+.email-circle {


Why is this called email circle...

Naming is important... I hope we can name it something that matches the description.

kjaymiller · 2024-10-28T14:06:33Z

tests/test.py

@@ -99,3 +99,16 @@ def test_mailto_bpdevs(page_url: tuple[Page, str]) -> None:
    page.goto(live_server_url)
    mailto = page.get_by_role("link", name="email")
    expect(mailto).to_have_attribute("href", "mailto:[email protected]")
+


Some tests that we have coded for that I would like to see.

Test 1:

Add a small post as a demo (in test as a test case and destroy it after)

Wait and let it rebuild and then check to see if the default image is grabbed.

Test 2

If you select the next_button you expect the image to change to a new one. Same for prev_button

Test 3

That slides match the links in the recent posts section

Test 4

When you click the links in the carousel they go to the corresponding webpages

Thanks Jay. I'll get to implementing these changes right away

TimOsahenru · 2024-10-30T07:30:44Z

@TimOsahenru I'm glad to hear that you were able to get the tests passing? What did you wind up doing in the end?

I tried a couple of unconventional things 😆, but ultimately increasing the server time from 4 to 20 was what did the trick for me.

TimOsahenru · 2024-10-30T07:39:45Z

@TimOsahenru,

This is a great start!

There are a couple of things that I would like to see refined before we go live with the changes. That said I don't want to lose this work so I've created a new branch called New-Carousel... can you submit the PR to that branch and then we can look at some of the things that I would hope that we can get touched up?

Quick List of things to address:

Only show the blog posts that are in recent posts. This also means that we can de-emphasize that section and make it smaller.

The circle image makes it hard to see some images, it may be better as a full width image`

With the new featured image setup, we should be able to use that for articles with

The descriptions seem like the first sentence of the content and not a dedicated line that can be used for MetaTagging as well. - I recently created a tool that would generate autodescriptions for my personal blog that could be re-used for this. - https://github.com/kjaymiller/kjaymiller.com/blob/main/ollama_summarizer.py `

The section is margined inside the container making it smaller than the content near it. We can fix that by changing the width of the container from 85% to 100%

The default image seems to be a little buggy. Instead of giving the default image it like creates an image of the raw content... we can fix this by removing default: post.content | split: '!' | first | split: '(' | last | split: ')' | first |

Thanks. I'll effect all these changes and make a push to New-Carousel branch

TimOsahenru added 6 commits October 26, 2024 19:48

Increased timeout from4 to 20 to give time to my server to start

83d2a39

Carousel added

5fed52f

Bootstrap imported

351a1e0

Styling added

16b968c

Added test for carousel slider

e5f1ca4

Commented code removed

b8d3d3b

kjaymiller requested changes Oct 28, 2024

View reviewed changes

kjaymiller changed the base branch from gh-pages to New-Carousel October 29, 2024 09:19

kjaymiller merged commit 2e54647 into BlackPythonDevs:New-Carousel Oct 29, 2024
2 checks passed

This was referenced Oct 30, 2024

replace hero image with carousel from latest posts and featured images of them. #365

Closed

Make changes to new carousel #553

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace hero image with carousel slider #544

Replace hero image with carousel slider #544

TimOsahenru commented Oct 26, 2024

kjaymiller commented Oct 28, 2024

kjaymiller commented Oct 28, 2024

kjaymiller left a comment

kjaymiller Oct 28, 2024

kjaymiller Oct 28, 2024

TimOsahenru Oct 30, 2024

kjaymiller Oct 28, 2024

kjaymiller Oct 28, 2024

TimOsahenru Oct 30, 2024

kjaymiller Oct 28, 2024

kjaymiller Oct 28, 2024

kjaymiller Oct 28, 2024

TimOsahenru Oct 30, 2024

TimOsahenru commented Oct 30, 2024

TimOsahenru commented Oct 30, 2024

Replace hero image with carousel slider #544

Replace hero image with carousel slider #544

Conversation

TimOsahenru commented Oct 26, 2024

Issue Link 🔗:

Type of Change

Description 📋

Checklist ✅

Additional Notes & Screenshots

kjaymiller commented Oct 28, 2024

kjaymiller commented Oct 28, 2024

kjaymiller left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Test 1:

Test 2

Test 3

Test 4

Choose a reason for hiding this comment

TimOsahenru commented Oct 30, 2024

TimOsahenru commented Oct 30, 2024