Skip to content

Replace hero image with carousel slider #544

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Oct 29, 2024
Merged

Replace hero image with carousel slider #544

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
83d2a39
Increased timeout from4 to 20 to give time to my server to start
TimOsahenru Oct 26, 2024
5fed52f
Carousel added
TimOsahenru Oct 26, 2024
351a1e0
Bootstrap imported
TimOsahenru Oct 26, 2024
16b968c
Styling added
TimOsahenru Oct 26, 2024
e5f1ca4
Added test for carousel slider
TimOsahenru Oct 26, 2024
b8d3d3b
Commented code removed
TimOsahenru Oct 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions _layouts/default.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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/bootstrap@5.3.0-alpha1/dist/css/bootstrap.min.css" rel="stylesheet">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

{% if page.lang and page.untranslated != true and site.data.locales.size > 1 %}
{% assign locales = site.data.locales | sort %}
{% for locale in locales %}
Expand All @@ -36,5 +37,6 @@
{%- include footer.html -%}
<script src="{{ '/assets/js/index.js' | relative_url }}"></script>
<script src="https://kit.fontawesome.com/1bbe56de49.js" crossorigin="anonymous"></script>
<script src="https://cdn.jsdelivr.net/npm/bootstrap@5.3.0-alpha1/dist/js/bootstrap.bundle.min.js"></script>
</body>
</html>
28 changes: 25 additions & 3 deletions _layouts/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,31 @@
---
<div class="home">
{% assign t = site.data.locales[page.lang][page.lang] %} {% if page.title %} {% assign header = page.title %} {% else %} {% assign header = site.title %} {% endif %}
<article class="hero">
<div class="hero-text">{{ t.index.lead }}</div>
</article>
<header>
<div id="postSlider" class="carousel slide slider-container" data-bs-ride="carousel">
<div class="carousel-inner">
{% 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 }}">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha! Working on it. Thanks

</div>
<h3 class="post-title">{{ post.title }}</h3>
<p class="post-summary">{{ post.excerpt | strip_html | truncatewords: 20 }}</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

</div>
{% endfor %}
</div>

<!-- Controls -->
<button class="carousel-control-prev" type="button" data-bs-target="#postSlider" data-bs-slide="prev">
<span class="carousel-control-prev-icon" aria-hidden="true"></span>
<span class="visually-hidden">Previous</span>
</button>
<button class="carousel-control-next" type="button" data-bs-target="#postSlider" data-bs-slide="next">
<span class="carousel-control-next-icon" aria-hidden="true"></span>
<span class="visually-hidden">Next</span>
</button>
</div>
</header>
{{ content }}

<img src="{{'/assets/images/dcus.jpg' | relative_url }}" alt="DjangoCon US 2023"/>
Expand Down
47 changes: 47 additions & 0 deletions assets/css/bpd.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,3 +517,50 @@ ul.speaking-list {
max-width: 60%;
}
}
header {
padding: 40px 20px;
margin-bottom: 20px;
/* background-color: black; */
}
.slider-container {
min-height: 300px;
display: flex;
justify-content: center;
align-items: center;
/* background-image: url("/assets/images/hero-bpd-background.jpg"); */
background-color: #222;
padding: 40px;
border-radius: 10px;
margin: 0 auto;
width: 85%;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

/* max-width: 1000px; */
}
.email-circle {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this called email circle...

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

width: 200px;
height: 200px;
background-color: #fff;
border-radius: 50%;
display: flex;
justify-content: center;
align-items: center;
margin: 0 auto 15px;
overflow: hidden;
}
.email-circle img {
width: 100%;
height: 100%;
object-fit: cover;
}
.carousel-item {
text-align: center;
}
.post-title {
font-size: 25px;
font-weight: bold;
color: white;
margin-bottom: 10px;
}
.post-summary {
font-size: 15px;
color: #f1f1f1;
}
15 changes: 14 additions & 1 deletion tests/test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def page_url(xprocess, url_port):
url, port = url_port

class Starter(ProcessStarter):
timeout = 4
timeout = 20
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

# Start the process
args = [
"bundle",
Expand Down Expand Up @@ -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:contact@blackpythondevs.com")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


def test_carousel_displayed(page_url: tuple[Page, str]) -> None:
page, live_server_url = page_url
page.goto(live_server_url)

carousel = page.locator(".carousel")
expect(carousel).to_be_visible()

next_button = page.locator(".carousel-control-next")
prev_button = page.locator(".carousel-control-prev")
expect(next_button).to_be_visible()
expect(prev_button).to_be_visible()
Loading