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

Changed backgroundcolor for interested on bdb interestpage #4232

Merged
merged 1 commit into from Oct 26, 2023
Merged

Conversation

ShaileshS1702
Copy link
Contributor

@ShaileshS1702 ShaileshS1702 commented Oct 24, 2023

reason: problems with darkmode

Resolves ... (either GitHub issue or Linear task)
ABA-615

@github-actions github-actions bot added the review-needed Pull requests that need review label Oct 24, 2023
@@ -114,7 +114,7 @@
}

.interested {
background-color: #f5f5d5;
background-color: #B33939;
Copy link
Member

Choose a reason for hiding this comment

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

Can we use a variable instead?😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we use a variable instead?😄

We thought about it. If we only change .interested and not the other classes: interested will change color when we change between light/dark mode, but the others will stay same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we use a variable instead?😄

We thought about it. If we only change .interested and not the other classes: interested will change color when we change between light/dark mode, but the others will stay same

one solution is to change multiple classes, but then the color at the page will be different and can maybe cause problems for bedroom

Should we still do it?

@ShaileshS1702 ShaileshS1702 added types Pull requests that improve or fix types small-fix Pull requests that fix something small labels Oct 24, 2023
@@ -19,6 +19,7 @@

--lego-max-width: 1100px;
--lego-default-padding: 2rem;
--bdb-interested: #B33939;
Copy link
Member

Choose a reason for hiding this comment

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

I think I prefer to use existing colours rather than introducing new ones

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The question is then what is the best way:
If we do choose to use variables, the color will change with the lightning mode:
image
image

While in light mode "ikke interessert" is darker than "Interessert", in dark mode the opposite can be observed. While this may not be a big issue, if the user of the site remember states based on color, it may be difficult if they change lightning.
We can either go through with or we can use a different color over all. The reason we chose to use red was that we thought it was smart that states that are similar should also have similar color.
As we are new to this, we are of course open to suggestions, so what do you think we should do?

Copy link
Contributor

Choose a reason for hiding this comment

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

@ivarnakken What's your opinion on adding color variables that don't change based on theme? It'd make the solution to issues such at these a lot easier.

Copy link
Member

Choose a reason for hiding this comment

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

"Ikke interessert" should be the danger color. "Interessert" shouldn't really be red at all, but rather a yellow/orange like you suggest.

The colors here in the bdb were picked randomly by a webkomer many many years ago. Feel free to change.

@ivarnakken ivarnakken added bug-fix Pull requests that fix a bug changes-requested Pull requests with requested changes small-fix Pull requests that fix something small and removed small-fix Pull requests that fix something small types Pull requests that improve or fix types labels Oct 24, 2023
reason: problems with darkmode

Co-authored-by: Magnus Brecke <magnus.brecke@gmail.com>
Co-authored-by: Christian Thielemann Grytøyr <christian.grytoyr@ebnett.no>
Copy link
Member

@ivarnakken ivarnakken left a comment

Choose a reason for hiding this comment

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

💯

@ivarnakken ivarnakken added approved Pull requests that have been approved and removed review-needed Pull requests that need review changes-requested Pull requests with requested changes labels Oct 26, 2023
@ivarnakken ivarnakken merged commit 9878841 into master Oct 26, 2023
4 checks passed
@ivarnakken ivarnakken deleted the atNye branch October 26, 2023 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Pull requests that have been approved bug-fix Pull requests that fix a bug small-fix Pull requests that fix something small
Projects
None yet
3 participants