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

Feature/bootstrap #63

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Feature/bootstrap #63

wants to merge 2 commits into from

Conversation

lukavdplas
Copy link
Contributor

Replaces bulma and primeNG with bootstrap / ng-bootstrap.

Also closes #52 (which was a bulma issue)

@lukavdplas lukavdplas requested a review from oktaal December 10, 2024 17:22
Copy link
Contributor

@oktaal oktaal left a comment

Choose a reason for hiding this comment

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

Looks great! Will need a few minor tweaks

*ngFor="let language of languages" (click)="setLanguage(language.code)">
<ul class="navbar-nav ms-auto">
<li class="nav-item">
<dh-dark-mode-toggle role="listitem" class="nav-item" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be {{cookiecutter.app_prefix}} instead of dh

<a [href]="environment.sourceUrl" target="_blank" i18n>Source code</a>&nbsp;<ng-container i18n>(BSD 3-Clause License)</ng-container>
</p>
<p>
<em><ng-container i18n>Version: </ng-container><span>{{"{{environment.version}}"}}<</span>&nbsp;(<span [title]="environment.buildTime">{{"{{buildTime}}"}})</span></em>
Copy link
Contributor

Choose a reason for hiding this comment

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

Extra <
image

<nav class="navbar navbar-expand-lg bg-body-tertiary" aria-label="main navigation"
data-bs-theme="dark">
<div class="container-fluid">
<div class="navbar-brand" i18n>
Copy link
Contributor

Choose a reason for hiding this comment

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

image

Aaaaaargh this is apparently standard Bootstrap (added the line to highlight the nope-nope-nope)

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add fs-6 fw-bold

@if (loading) {
<div class="spinner-border">
<span class="visually-hidden">Loading languages...</span>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

This becomes big when toggling:

image

Instead of <div class="spinner-border">...</div> use:

<span class="spinner-border spinner-border-sm" aria-hidden="true"></span>
<span class="visually-hidden" role="status">Loading...</span>

$yellow: #FFCD00,
$blue: #5287C6);
@use 'bulma/bulma';
@use 'bootstrap';
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps something like

// University Brand Colors
$primary: #FFCD00;
$green: #24A793;
$success: $green;
$red: #C00A35;
$danger: $red;
$yellow: #FFCD00;
$warning: $yellow;
$blue: #5287C6;
$info: $blue;
$link-color: $blue;
$link-color-dark: $blue;

@import "bootstrap/scss/bootstrap.scss";

To not lose the UU colors (the yellow in the menu is quite something though)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Text contrast
2 participants