Skip to content

feat: Implement enhancements for new design #192

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.

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

Open
wants to merge 12 commits into
base: feat/new-design
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions assets/sass/navigation.scss
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
.o-header__wrapper {
display: flex;
justify-content: space-between;
padding: 0;

@media (min-width: #{$breakpoint-md}) {
justify-content: unset;
Expand Down Expand Up @@ -56,9 +57,8 @@

.o-header__logo {
display: flex;
margin: .6rem;
margin: .6rem 0;
position: relative;
left: -1.6rem;
text-decoration: none;
border-radius: var(--border-radius-m);

Expand Down
54 changes: 53 additions & 1 deletion assets/sass/styles.scss
Original file line number Diff line number Diff line change
Expand Up @@ -180,11 +180,20 @@ img {
}

.o-startpage .row {
margin-bottom: 3.2rem;
margin-bottom: 2.4rem;
}

.o-startpage__intro {
text-wrap: balance;
margin-bottom: 2rem;
display: grid;
grid-template-columns: 1fr 1fr;
gap: 2rem;

@media (max-width: #{$breakpoint-md}) {
display: flex;
flex-direction: column;
}
Comment on lines +188 to +196
Copy link
Member

Choose a reason for hiding this comment

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

What added value does it have compared to Bootstrap?

Copy link
Member Author

Choose a reason for hiding this comment

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

I see a few advantages:

  • Reduced bundle size when removing Bootstrap completely, initially discussed here: Check necessity of bootstrap #74 (comment). If we still want to get rid of Bootstrap, I'd slowly decrease the dependency and at least leave it out from newly introduced code.
  • Native DevTools support. Chrome and Firefox have some neat development tools around grid layout. The devtools didn't work with the bootstrap grid layout.
  • Less chance of breaking changes. Bootstrap is under active development and can introduce breaking changes. CSS itself is considered as rather stable.

Copy link
Member

Choose a reason for hiding this comment

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

According to my understanding, the discussion is about using bootstrap not as an depency, but as local css rules. If you want to remove bootstrap in total, we should do this in a separate branch and not accorded to this feature branch. There are many more occurencies, which have to be replaced either.

Copy link
Member Author

Choose a reason for hiding this comment

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

I just thought that it makes sense to avoid Bootstrap for newly added content to reduce effort for the migration. But since I've done it in a separate commit anyway, I'll take it out and move it into a separate PR.


h2 {
font-family: Sansita,Arial,Helvetica,sans-serif;
Expand All @@ -196,4 +205,47 @@ img {
> div {
align-content: center;
}

@media (max-width: #{$breakpoint-md}) {
h2 {
font-size: 4.8rem;
}
}
}

.o-startpage__image {
@media (max-width: #{$breakpoint-md}) {
display: none;
}
}

.o-startpage__shortcut-wrapper {
display: grid;
grid-template-columns: 1fr 1fr 1fr 3fr;
gap: 2rem;
margin-bottom: 2rem;

@media (max-width: #{$breakpoint-lg}) {
display: flex;
flex-wrap: wrap;
gap: 1rem;
}

}

.o-startpage__search {
margin-bottom: 2rem;
width: 100%;
}

.o-startpage__news-teaser-wrapper {
display: grid;
grid-template-columns: 1fr 1fr 1fr;
gap: 2rem;

@media (max-width: #{$breakpoint-md}) {
display: flex;
flex-direction: column;
}

}
60 changes: 36 additions & 24 deletions assets/sass/teaser.scss
Original file line number Diff line number Diff line change
Expand Up @@ -10,44 +10,35 @@
border-radius: var(--border-radius-l);
display: flex;
flex-direction: column;

&:nth-child(-n+3) {
@media (max-width: #{$breakpoint-lg}) {
margin-bottom: 2rem;
}
}
}

.m-teaser--listview .m-teaser__wrapper {
margin-bottom: 2.4rem;
flex-direction: row;
display: grid;
grid-template-columns: 1fr 2fr;

@media (max-width: #{$breakpoint-md}) {
display: flex;
flex-direction: column;
}

.m-teaser__image {
@media (min-width: #{$breakpoint-md}) {
max-width: 33%;
}

img {
border-radius: var(--border-radius-l) 0 0 var(--border-radius-l);
.m-teaser__image img {
border-radius: var(--border-radius-l) 0 0 var(--border-radius-l);

@media (max-width: #{$breakpoint-md}) {
border-radius: var(--border-radius-l) var(--border-radius-l) 0 0;
width: 100%;
}
@media (max-width: #{$breakpoint-md}) {
border-radius: var(--border-radius-l) var(--border-radius-l) 0 0;
width: 100%;
}
}
}

.m-teaser--shortcut-tile {
flex-basis: calc( 50% - 0.5rem );

.m-teaser__wrapper {
width: 100%;
height: 12rem;
padding: .8rem;
flex-direction: row;
height: 11rem;
padding: 1.4rem;
justify-content: space-between;

span {
Expand All @@ -59,16 +50,29 @@

a {
h3 {
text-decoration: underline;
align-self: center;
overflow-wrap: normal;
word-break: normal;
white-space: normal;
hyphens: manual;
margin-bottom: 0;
}

transition: background 0.2s, color 0.2s;

&:hover,
&:focus {
background: var(--link-hovered);
border-color: var(--link-hovered);
color: #fff;
}
Comment on lines +62 to +67
Copy link
Member

Choose a reason for hiding this comment

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

  1. the color change is too heavy in my opinion.
  2. It's important to me that we have consistency. In my version, the headline is underlined. If we want to change that, we should apply it to all teasers, not just the shortcut tiles.

Copy link
Member Author

Choose a reason for hiding this comment

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

  1. That's a fair point. @lenderom already changed it for the button where the same color change was in place. I'd wait until the booking expander PR is merged and then rebase this PR on it.
  2. I agree that we should keep it consistent. There are a few inconsistencies that we should address:
    1. On the teaser, users can only click on the link, while for the links the whole box is clickable. In my opinion, we should make all parts of the teaser clickable.
    2. For the links we have the arrow icon in on the bottom right which we don't have for the teasers. However, I don't think that the arrows would fit to the teasers, so it's fine for me.
    3. Having the links always underlined looks a bit dated to me, so I'd prefer to change it to underline on hover only. This would then also be consistent with the expanders where we also only underline on hover.

What's your opinion here?

Copy link
Member

Choose a reason for hiding this comment

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

My concerns are mostly about accessibility and usability. The teasers must look clickable and have to be accessible. What do you think of this suggestion:

  1. All teasers use the hover effect of the buttons in feat: Present booking methods as custom expanders #180
  2. To get a clickable look all teasers use the arrow icon, in the bottom left corner (like bahnhof.de for example)
  3. in default no teaser headline is underlined, just on hover/focus (like the expanders)
  4. on hover the whole card gets the hover effect (with ::after Element of the link)
  5. on focus the headline is becoming the focus indicator

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good, let's do it!

}
}

.m-teaser__image img {
aspect-ratio: 16/9;
border-radius: var(--border-radius-l) var(--border-radius-l) 0 0;
object-fit: cover;
height: 100%;
Comment on lines +74 to +75
Copy link
Member

Choose a reason for hiding this comment

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

in my opinion, we should show the images always in aspect-ratio 16/9. Don't you think so?

Copy link
Member Author

Choose a reason for hiding this comment

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

I disagree here :)

Let's have a look at the news list with a viewport that has a width of 850px.

This is the previous version:
image

I can see mainly two issues here:

  1. The images have a small white line at the bottom because they can't fit properly.
  2. The text preview is very limited.

Number 1 is probably easily fixable. Number 2 is more challenging: When I increase the preview lines, the height increases. When we would preserve the image ratio now, the width would also increase and takes away the space that I would use for the content preview.

In the updated version, I have three preview lines. The aspect ratio is not 16:9 anymore, but because of the CSS object-fitting, it just zooms in slightly. Still looks good in my opinion.

image

Copy link
Member

Choose a reason for hiding this comment

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

These are two separate aspects of it:

  1. Using 16/9 aspect ratio
  2. Prevention of whitespace

Do you think, there is no way / no need to keep 16/9? Otherwise we lose control over the shown part of the image :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think that there is a necessity to have an image ratio of 16/9. I would rather formulate the requirement as "The images should be displayed in a 16/9 ratio", so try to make it work where it works out. Most of the images don't depend on a specific parts of the image.

I believe we're talking about a user group of less than 5% anyway (only users with a screen width between 768px and 992px see the images in a another screen ratio than 16/9). For those users, we have one of the following options:

  1. Only show a part of the image like it's the case in my proposal.
  2. Don't show the image at all for the specific screen width range.
  3. Reduce the text to display (not three lines, but make an exception for the specific screen width range) like it's the case in your proposal, but would need some refinement to avoid the white space.

What do you think about the proposal to keep number 1, but take it as discussion point to the next meeting. Then we can also hear the other opinions and come to a conclusion. I'd also implement whatever we decide on.

}

.m-teaser__dateline {
Expand All @@ -80,5 +84,13 @@
}

.m-teaser__content {
padding: .8rem;
padding: 1.4rem;
}

.m-teaser__text {
line-clamp: 3;
display: -webkit-box;
-webkit-line-clamp: 3;
-webkit-box-orient: vertical;
overflow: hidden;
}
2 changes: 1 addition & 1 deletion content/operator/_index.de.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
title: "Übersicht der Betreiber mit FIP"
title: "Übersicht der Betreiber"
description: "Übersicht über die Betreiber, die FIP-Vergünstigungen anbieten."
---

Expand Down
2 changes: 1 addition & 1 deletion content/operator/_index.en.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
title: "Overview of operators supporting FIP"
title: "Overview of operators"
description: "Overview of the operators providing FIP benefits."
---

Expand Down
2 changes: 1 addition & 1 deletion layouts/_default/baseof.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
dir="{{ or site.Language.LanguageDirection `ltr` }}"
>
<head>
{{ partial "head.html" . }}
{{ partial "head" . }}
</head>
<body>
<header id="header" class="o-header">{{ partialCached "header.html" . }}</header>
Expand Down
32 changes: 15 additions & 17 deletions layouts/_default/home.html
Original file line number Diff line number Diff line change
@@ -1,39 +1,37 @@
{{ define "main" }}
<h1 class="sr-only">{{ T "home-page-text" }}</h1>
<section class="container">
<div class="row o-startpage__intro">
<div class="col-12 col-lg-6 order-2 order-lg-1">
<div class="o-startpage__intro">
<div>
{{ .Content }}
</div>
<div class="col-12 col-lg-6 order-1 order-lg-2">
<div class="o-startpage__image">
{{ $image := resources.Get "images/startpage.png" }}
{{ partial "image.html" (dict "image" $image "eager" true) }}
{{ partial "image" (dict "image" $image "eager" true) }}
</div>
</div>
<div class="row">
<div class="col-12">
{{ partial "search-slot" }}
</div>

<div class="o-startpage__search">
{{ partial "search-slot" }}
</div>
<div class="row">

<div class="o-startpage__shortcut-wrapper">
{{ partial "shortcut-tile" (.Site.GetPage "generalinformation") }}
{{ partial "shortcut-tile" (.Site.GetPage "country") }}
{{ partial "shortcut-tile" (.Site.GetPage "operator") }}
</div>
</section>
<section class="container">
<div class="row">
<div class="col-12">
{{ (site.GetPage "_supportUs").Content }}
</div>
{{ (site.GetPage "_supportUs").Content }}
</div>
</section>
<section class="container">
<div class="row justify-content-center">
<h2>{{ T "news-headline"}}</h2>
{{ range first 3 (where site.RegularPages "Section" "news") }}
{{ partial "teaser.html" (dict "listview" false "type" "news" "page" .) }}
{{ end }}
<h2>{{ T "news-headline"}}</h2>
<div class="o-startpage__news-teaser-wrapper">
{{ range first 3 (where site.RegularPages "Section" "news") }}
{{ partial "teaser" (dict "listview" false "type" "news" "page" .) }}
{{ end }}
</div>
</section>
<div class="curtain" aria-hidden="true"></div>
Expand Down
2 changes: 1 addition & 1 deletion layouts/_default/rss.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<title>{{ .Title }}</title>
<link>{{ .Permalink }}</link>
<!-- Remove the icons and XML escape the summary -->
{{ $cleanedHTML := .Summary | replaceRE `<span[^>]*class="material-symbols-rounded"[^>]*>.*?</span>` "" | transform.XMLEscape | safeHTML}}
{{ $cleanedHTML := partial "strip-material-icons" .Summary | transform.XMLEscape | safeHTML}}
<description>{{ $cleanedHTML }}</description>
<pubDate>{{ .PublishDate.Format "Mon, 02 Jan 2006 15:04:05 -0700" | safeHTML }}</pubDate>
<enclosure url="{{ (partial "helper/contentImage" . ).Permalink }}" />
Expand Down
2 changes: 1 addition & 1 deletion layouts/news/list.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ <h1 data-pagefind-meta="title">{{ .Title }}</h1>
{{ .Content }}
<div class="row">
{{ range .Pages }}
{{ partial "teaser.html" (dict "listview" "true" "page" .) }}
{{ partial "teaser" (dict "listview" "true" "page" .) }}
{{ end }}
</div>
</div>
Expand Down
6 changes: 3 additions & 3 deletions layouts/partials/head.html
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
{{ partial "headMeta.html" . }}
{{ partial "headMeta" . }}

<title>
{{ if .IsHome }}{{ site.Title }}{{ else }}{{ printf "%s | %s" .Title site.Title }}{{ end }}
</title>
{{ partialCached "head/js.html" . }}
{{ partialCached "head/js" . }}
{{ $options := (dict "targetPath" "css/styles.css" "outputStyle" "compressed") }}
{{ with resources.Get "sass/main.scss" }}
{{ $style := (resources.ExecuteAsTemplate "sass/main_templated.scss" $ .) | toCSS $options | minify }}
Expand Down Expand Up @@ -43,4 +43,4 @@
{{- end -}}
<link rel="alternate" hreflang="x-default" href="{{ .Site.BaseURL }}" />

{{ partial "structuredData.html" . }}
{{ partial "structuredData" . }}
2 changes: 1 addition & 1 deletion layouts/partials/header.html
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{{ partial "menu.html" (dict "menuID" "main" "page" .) }}
{{ partial "menu" (dict "menuID" "main" "page" .) }}
2 changes: 1 addition & 1 deletion layouts/partials/shortcut-tile.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<div class="m-teaser m-teaser--shortcut-tile col-6 col-sm-4 col-lg-3 col-xl-2">
<div class="m-teaser m-teaser--shortcut-tile">
<a href="{{ .RelPermalink }}" class="m-teaser__wrapper">
<h3>{{ .Title }}</h3>
{{- partial "icon" "arrow_forward" -}}
Expand Down
2 changes: 1 addition & 1 deletion layouts/partials/sidemenu.html
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
{{ end }}

<h3 id="toc">{{ .Title }}</h3>
{{ partial "toc.html" (dict "context" . "startLevel" 2 "endLevel" 3 ) }}
{{ partial "toc" (dict "context" . "startLevel" 2 "endLevel" 3 ) }}

{{ if eq .Page.Type "country" }}
{{ partial "related" (dict "index" "country" "pageType" "operator" "title" (T "_operator__list_title") "page" . "identifier" "operators") }}
Expand Down
1 change: 1 addition & 0 deletions layouts/partials/strip-material-icons.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{{ return replaceRE `<span[^>]*class="[^"]*\bmaterial-symbols-rounded\b[^"]*"[^>]*>.*?</span>` "" . }}
4 changes: 2 additions & 2 deletions layouts/partials/teaser.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

{{ $dateMachine := $page.Date | time.Format "2006-01-02T15:04:05-07:00" }}
{{ $dateHuman := $page.Date | time.Format ":date_long" }}
<div class="m-teaser{{ if $listview }} m-teaser--listview{{ else }} col-lg-4 col-md-6{{ end }} col-12">
<div class="m-teaser{{ if $listview }} m-teaser--listview{{ end }}">
<div class="m-teaser__wrapper">
{{ $image := (partial "helper/contentImage" $page ) }}
{{ if $image }}
Expand All @@ -22,7 +22,7 @@ <h3 class="m-teaser__headline">{{ $page.Title }}</h3>
</a>

<div class="m-teaser__text">
{{ $page.Summary | plainify | truncate 100 }}
{{ partial "strip-material-icons" $page.Summary | plainify | truncate 500 }}
</div>
</div>
</div>
Expand Down
2 changes: 1 addition & 1 deletion layouts/partials/toc.html
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@

@returns {template.HTML}

@example {{ partial "toc.html" (dict "context" . "startLevel" 2 "endLevel" 3 ) }}
@example {{ partial "toc" (dict "context" . "startLevel" 2 "endLevel" 3 ) }}
*/}}

{{- /* Initialize. */}}
Expand Down
Loading