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

Basic home page with palceholders #1

Merged
merged 7 commits into from
Jan 27, 2023
Merged

Basic home page with palceholders #1

merged 7 commits into from
Jan 27, 2023

Conversation

EmilyBoarer
Copy link
Collaborator

No description provided.

@EmilyBoarer EmilyBoarer requested a review from mxbi January 27, 2023 18:05
@EmilyBoarer EmilyBoarer merged commit 6fe7d15 into main Jan 27, 2023
@EmilyBoarer EmilyBoarer deleted the home_page_eb branch January 27, 2023 18:13
Copy link
Member

@mxbi mxbi left a comment

Choose a reason for hiding this comment

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

Looks good, couple changes for next round

@@ -3,9 +3,37 @@
<head>
{% include "includes/header.html" %}
<meta charset="UTF-8">
<title>Title</title>
<title>jacob.covs.tld</title> <!-- TEMP:REPLACE -->
Copy link
Member

Choose a reason for hiding this comment

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

We'll need to make some config file that constants like this get pulled from. OK for now

<title>Title</title>
<title>jacob.covs.tld</title> <!-- TEMP:REPLACE -->

<div class="container">
Copy link
Member

Choose a reason for hiding this comment

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

Pull out navbar into templates/includes/navbar.html, which we can then do {% include "includes/navbar.html" %} within each page.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was going to, but that assumes that the navbar will be the same on each page, which it won't be:
The current page's highlighting needs changing, and which options are available need to be changed.
Would this still work anyway (i.e. can we dynamically change things in the included navbar.html ?) ??

<!-- Bootstrap: -->
<meta name="viewport" content="width=device-width, initial-scale=1">
<link href="https://cdn.jsdelivr.net/npm/bootstrap@5.3.0-alpha1/dist/css/bootstrap.min.css" rel="stylesheet" integrity="sha384-GLhlTQ8iRABdZLl6O3oVMWSktQOp6b7In1Zl3/Jr59b6EGGoI1aFkw7cmDA6j6gD" crossorigin="anonymous">
Copy link
Member

Choose a reason for hiding this comment

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

+1 for adding the hash

@EmilyBoarer EmilyBoarer restored the home_page_eb branch January 27, 2023 18:25
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.

2 participants