Skip to content
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

Add Dark Theme #581

Open
wants to merge 1 commit into
base: main
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
2 changes: 1 addition & 1 deletion myhpi/search/templates/search/search_result.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ <h5 class="text-break">{{ result.title }}</h5>
{{ result.specific.date|date:"SHORT_DATE_FORMAT" }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Adding comments to random lines to make them resolvable one by one

Is it possible to remove the border (or make it same color as the highlight) on hover?

For that to work, we might need a more significant contrast between the hover highlight and the background color

This design:
grafik

Old design:
grafik

</time>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it intentionally that the divider in the user menu seems to be gone?

Old menu for comparison:
grafik

</hgroup>
Copy link
Collaborator

Choose a reason for hiding this comment

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

The mobile menu needs its background back ^^

grafik

<span class="text-dark">
<span class="search-preview">
{% if result.search_description %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

The border at the bottom of the menu seems to be gone, which sometimes makes it hard to differentiate it from the background:

grafik

This is the old design:

grafik

{{ result.search_description }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

The input elements now seems to be gray by default, which looks like they would be disabled. Could we make them white again?

grafik

grafik

{% else %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

The selection in the results page now looks a bit off when pressing an item, due to now making the selection darker but keeping the same border and making the background white again

This PR:

grafik

Production:

grafik

Expand Down
87 changes: 87 additions & 0 deletions myhpi/static/img/myHPI-Logo-white.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion myhpi/static/scss/footer.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
.footer-myhpi {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it wanted that there is no difference between the webpage background and the content background? The orange bar at the top and the footer at the bottom look a little bit lost to me without the content background

grafik

This might also fix the lack of contrast between table cells and content background:

grafik

background-color: var(--bs-gray-700);
Copy link
Collaborator

Choose a reason for hiding this comment

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

The breadcrumbs chevrolets could be white

grafik

background-color: var(--bs-gray-800);

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a way to increase the contrast? Firefox throws a contrast warning

grafik

.page-content {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The border between top bar and content is not visible any more. For better distinction, could we introduce it again?

This is how it looked:

grafik

padding-top: 1.5rem;
Copy link
Collaborator

Choose a reason for hiding this comment

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

A difference between the menu and background color would be helpful to differentiate both better (now they flow into each other):

grafik

Before:

grafik

Especially helpful in the dark mode:

grafik

Expand Down
53 changes: 40 additions & 13 deletions myhpi/static/scss/myHPI.scss
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,25 @@
$primary: #F5631E;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Making the breadcrumbs gray again could help differentiating them from the page content:

grafik

Before:
grafik

$secondary: #F5631E;
Copy link
Collaborator

Choose a reason for hiding this comment

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

In [myhpi/core/templates/core/sidebar.html](https://github.com/fsr-de/myHPI/pull/525/files/bc3f7996429831f3857afe63e7b344cfd023d219#diff-8c66ac3d840d267494b184c335c50d73e56b70724f454183f3af2b86242632a7), there is the following comment:

<!-- Bootstrap 5.2 supports responsive offcanvas. This duplication is a workaround and should be removed after the upgrade -->

Could be addressed now, since we updated to 5.3

$body-bg: #F5F5F5;
$body-color-dark: white;
$body-color: black;

$gray-100: #e9e9e9;
$gray-200: #EEE;
$gray-300: #DDD;
$gray-400: #CCC;
$gray-500: #BBB;
$gray-600: #999;
$gray-700: #777;
$gray-800: #555;
$gray-900: #333;

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it wanted that there is no difference between the webpage background and the content background? The orange bar at the top and the footer at the bottom look a little bit lost to me without the content background

grafik

This might also fix the lack of contrast between table cells and content background:

grafik

// 3. Include remainder of required Bootstrap stylesheets
@import "./bootstrap/variables";
@import "./bootstrap/mixins";
@import "./bootstrap/maps";

$custom-colors: (
"gray-700": $gray-700,
"text-muted": $text-muted,
"border": #EEE,
"border-dark": #B2B2B2,
"body-bg-light": #FBFBFB,
"body-bg-overlay": #EEE
);
$colors: map-merge($colors, $custom-colors);
$color-mode-type: media-query;

@import "./bootstrap/root";

Expand Down Expand Up @@ -62,10 +67,24 @@ $colors: map-merge($colors, $custom-colors);

// 6. Add additional custom code here



:root {
--bs-body-bg-overlay: var(--bs-gray-200);
--bs-p-2: 0.5rem;
--bs-p-4: 1rem;
--myhpi-round-border: 2px;
--content-bg: white;
--bs-border-dark: var(--bs-gray-500);
}

@include color-mode(dark) {
:root {
--bs-body-bg-overlay: var(--bs-gray-700);
--bs-breadcrumb-divider-color: var(--bs-gray-500);
--content-bg: var(--bs-dark-bg);
--bs-border-dark: var(--bs-gray-600);
}
}

a {
Expand Down Expand Up @@ -121,7 +140,7 @@ p {

a,
a:hover {
color: var(--bs-text-muted);
color: var(--bs-body-color);
text-decoration: none;
}

Expand Down Expand Up @@ -186,6 +205,10 @@ h1.side-panel-title::after {
padding: 0;
margin: -0.5rem -0.8rem;

.search-preview {
color: var(--bs-body-color);
}

.search-result {
h5 {
hyphens: auto;
Expand All @@ -200,13 +223,13 @@ h1.side-panel-title::after {

&:hover,
&:focus-visible {
border-color: var(--bs-border);
background-color: var(--bs-body-bg-light);
border-color: var(--bs-border-dark);
background-color: var(--bs-light-bg-subtle);
}

&:active {
border-color: var(--bs-border-dark);
background-color: var(--bs-body-bg-overlay);
background-color: var(--bs-light-bg-overlay);
}
}
}
Expand Down Expand Up @@ -328,3 +351,7 @@ img {
padding: 3px;
}
}

#page {
background-color: var(--content-bg)
}
14 changes: 8 additions & 6 deletions myhpi/static/scss/navbar.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
:root {
--logo-width: 6.5rem;
--logo-margin: 1rem;
--bs-nav-link-padding-x: 1rem;
--bs-nav-link-padding-y: 0.5rem;
}

.navbar-myhpi {
Expand All @@ -20,13 +22,13 @@

.nav-link,
.nav-link:focus {
color: var(--bs-gray-700);
color: var(--text-muted);
}

.nav-link:hover,
.nav-link:focus-visible,
.nav-link[aria-expanded=true] {
color: var(--bs-dark);
color: var(--test-muted);
}

.nav-item-container {
Expand Down Expand Up @@ -86,7 +88,7 @@
}

.navbar-bottom {
background-color: var(--bs-body-bg-light);
background-color: var(--bs-light-bg-subtle);

.nav-item-container {
padding-left: var(--bs-p-4);
Expand All @@ -107,13 +109,13 @@
.nav-link {
transition: all 0.25s;
border-radius: var(--myhpi-round-border);
border: solid 2px var(--bs-body-bg-light);
border: solid 2px transparent;
}

.nav-link:hover,
.nav-link:focus-visible,
.nav-link:active {
border-color: var(--bs-body-bg-overlay);
border-color: var(--bs-border-dark);
background-color: var(--bs-body-bg-overlay);
}

Expand Down Expand Up @@ -213,4 +215,4 @@
transform: rotateZ(180deg) !important;
}
}
}
}
10 changes: 7 additions & 3 deletions myhpi/templates/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,15 @@
{% wagtailuserbar %}

<nav id="navbar" class="navbar-myhpi page d-flex flex-column d-print-none container-lg fixed-top xl-hide-on-scroll">
<div class="navbar-top bg-white d-flex">
<div class="navbar-top d-flex">
<div class="page-content d-flex flex-grow-1 flex-xl-grow-0">
<div id="navbar-left" class="d-flex align-items-center">
<a class="nav-brand" href="{% pageurl root_page %}">
<img src="{% static 'img/myHPI-Logo.svg' %}" class="w-100" alt={% translate "To home page" %}>
<picture class="w-100" alt={% translate "To home page" %}>
<source srcset="{% static 'img/myHPI-Logo.svg' %}" media="(prefers-color-scheme: light)"/>
<source srcset="{% static 'img/myHPI-Logo-white.svg' %}" media="(prefers-color-scheme: dark)"/>
<img src="{% static 'img/myHPI-Logo.svg' %}"/>
</picture>
</a>
</div>
<div id="nav-level-0" class="nav-level"></div>
Expand Down Expand Up @@ -129,7 +133,7 @@
</div>
</nav>

<div id="page" class="container-lg page bg-white pb-3">
<div id="page" class="container-lg page pb-3">
<div class="page-content">
<div id="messages">
{% for message in messages %}
Expand Down
2 changes: 1 addition & 1 deletion tools/install_bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

TEMP_DIRECTORY = "tmp"
ZIP_FILENAME = "bootstrap.zip"
VERSION = "5.0.2"
VERSION = "5.3.3"
logger = logging.getLogger("myhpi_install_bootstrap")


Expand Down
Loading