-
-
Notifications
You must be signed in to change notification settings - Fork 43
Change navbar color to #f6d9ef. Related to Issue #4 #93
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
Conversation
Dockerfile
Outdated
@@ -19,7 +19,8 @@ FROM base AS dev | |||
ARG USER_ID | |||
ARG GROUP_ID | |||
|
|||
RUN groupadd -o -g $GROUP_ID -r usergrp | |||
# RUN groupadd -o -g $GROUP_ID -r usergrp # original | |||
RUN groupadd -g $GROUP_ID usergrp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm this breaks when I try running make serve
after checking out your PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the feedback! I ran the Docker file in a Windows environment using WSL and found that my machine couldn't automatically read the USER_ID and GROUP_ID, so I had to modify my this file slightly to ensure my machine could read USER_ID and GROUP_ID and run properly.
I believe the mistake I made was sending environment-related files to the repository. They should be added to the .gitignore instead. I will restore this ASAP!
Please correct me if I misunderstood anything. 😃
compose.yml
Outdated
@@ -12,7 +12,7 @@ services: | |||
POSTGRES_FSYNC: null | |||
healthcheck: | |||
test: ["CMD", "pg_isready", "-U", "pyladiescon", "-d", "pyladiescon"] | |||
interval: 1s | |||
interval: 5s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason for updating this value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a similar situation as the one above. I'll fix it up!
compose.yml
Outdated
@@ -27,6 +27,9 @@ services: | |||
web: | |||
build: | |||
target: dev | |||
args: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason for updating these values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a similar situation as the one above. I'll fix it up!
templates/portal/base.html
Outdated
@@ -21,7 +21,8 @@ | |||
<body> | |||
{% block body %} | |||
{% endblock %} | |||
<nav class="navbar navbar-expand-md navbar-dark bg-dark" aria-label="Fourth navbar example"> | |||
<!-- <nav class="navbar navbar-expand-md navbar-dark bg-dark" aria-label="Fourth navbar example"> --> | |||
<nav class="navbar navbar-expand-md navbar-light" style="background-color: #f6d9ef;" aria-label="Fourth navbar example"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR.
I'm uncomfortable with adding hardcoded css color like this.
I think a good approach would be looking into developing a customizable theme. The actual individual colors are not as important, eg we don't need to decide on which shade of pink right now. What would be important is to have the values stored as variables, so that when we decide to change the color, we don't need to go through all the different html files to adjust the color, but simply adjusting some variables in the css file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After some exploration, I found two better solutions than using hardcoded, inline CSS. First, if it's a dynamic website, it would be better to modify the SCSS file in the Bootstrap framework. Second, if it's a static website, we can simply customize the CSS file.
Since some forms are embedded in our HTML file, it would be better to adjust the SCSS file, right?
I'll fix it by the end of this weekend!
Hello!! @Mariatta |
Fixes #4
Hello!! @Mariatta
I was working on Issue #4 and would like to discussion on the background and text color choices.
Here's the experiment I did:
Could you please provide some feedback on this? I'm interested in knowing more about whether the color works well with the rest of the design, or if any adjustments are needed.
Thank you!