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

Light Theme #13

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open

Light Theme #13

wants to merge 31 commits into from

Conversation

nomisum
Copy link
Contributor

@nomisum nomisum commented May 9, 2024

  • added custom fonts
  • added base theme colors
  • changed menu entry highlight
  • rounded images

todo:

  • dark theme
  • optimization of colored surfaces
  • question olive as link color, contrast issues

@nomisum nomisum requested a review from y0014984 May 9, 2024 19:50
Copy link
Contributor

@y0014984 y0014984 left a comment

Choose a reason for hiding this comment

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

Sorry, but I think this PR needs some changes before merging to master.

  • Most important: the light/dark theme switcher is now broken. It seems that the light theme is your intended theme but if I switch to dark theme then the background colors stay the same and the text colors get light. In this mode the texts aren't readable anymore.
  • Do you have chosen a different background color in comparison to arma.events to allow some kind of contrast between the arma.events screenshots and the background in docs.arma.events? Perhaps there are other solutions to that problem like a border around every image or a drop shadow.
  • The font weight is very thin. It's a bit hard to read. Could you increase font weight?
  • The green font in code blocks has an even harder readability. Perhaps increasing font weight globally would solve this issue too.
  • The round image corners look good. But there are missing for videos. See 5 videos in the slotlist article.

@y0014984
Copy link
Contributor

Looks much better now.

@nomisum
Copy link
Contributor Author

nomisum commented May 10, 2024

@y0014984 can you check again if you find obvious mistakes?

@y0014984
Copy link
Contributor

@nomisum All in all it looks quite good. But some final annotations:

Is it intentional to leave the blue buttons on the start screen in blue color (http://localhost:5173/en/)?

image

The black bar of the arma.events logo is barely visible in dark mode on typical pages. Light mode uses a darkened box. Perhaps dark mode could you a lightened box.

image
image

We could increase the headline of the warning boxes. It's currently very similar to the copy text of the box. Perhaps a more bold font style would help.

image

Drop shadow around smaller tables looks weird.

image

@nomisum
Copy link
Contributor Author

nomisum commented May 10, 2024

@nomisum All in all it looks quite good. But some final annotations:

Is it intentional to leave the blue buttons on the start screen in blue color (http://localhost:5173/en/)?

yes and no, i need an exception for this. brand color 2 is used here and to make them green i dont want to add another green as brand color (as blue makes sense in other contexts). its solvable though.

image

The black bar of the arma.events logo is barely visible in dark mode on typical pages. Light mode uses a darkened box. Perhaps dark mode could you a lightened box.

not sure what this is referring to, the divider? i prefer it to be subtle but could be nudged up a tiny bit.

image
image

We could increase the headline of the warning boxes. It's currently very similar to the copy text of the box. Perhaps a more bold font style would help.

its actually intentional to make them less large and dominant as they carry little information. is it possible to change the title content in markdown? the default text is kind of useless.

image

Drop shadow around smaller tables looks weird.

hard to solve as large tables profit. we might nudge the intensity down.

image

@nomisum
Copy link
Contributor Author

nomisum commented Nov 23, 2024

i've updated the colors to reflect the actual platform colors. can you give it a sanity check @y0014984 ?

@y0014984

This comment was marked as resolved.

@y0014984

This comment was marked as outdated.

@y0014984

This comment was marked as outdated.

@y0014984

This comment was marked as resolved.

@y0014984

This comment was marked as outdated.

@y0014984

This comment was marked as resolved.

@y0014984

This comment was marked as resolved.

@y0014984

This comment was marked as resolved.

@nomisum nomisum requested a review from y0014984 November 24, 2024 16:27
@nomisum
Copy link
Contributor Author

nomisum commented Nov 24, 2024

alles comments addressed.

btw why is the crown blue instead of yellow in the table? 🤔

@y0014984
Copy link
Contributor

btw why is the crown blue instead of yellow in the table? 🤔

I don't know why. I think I used the highlight color from the original theme. We need to import the icon font from arma.events which allows us to easily use the icons. Currently, the crown is only an inline image.

y0014984

This comment was marked as resolved.

@nomisum nomisum requested a review from y0014984 November 24, 2024 17:08
y0014984

This comment was marked as resolved.

@nomisum nomisum requested a review from y0014984 November 27, 2024 19:56
.vitepress/config.mts Outdated Show resolved Hide resolved
y0014984

This comment was marked as resolved.

y0014984

This comment was marked as resolved.

y0014984

This comment was marked as resolved.

y0014984

This comment was marked as resolved.

Copy link
Contributor

@y0014984 y0014984 left a comment

Choose a reason for hiding this comment

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

Perhaps we could switch to round avatars instead of rectengular with round corners. That would be more common.

image

EDIT: This doesn't work because it's regular markdown and the Avatars are treated like every other image.

Copy link
Contributor

@y0014984 y0014984 left a comment

Choose a reason for hiding this comment

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

See my comments above. The is still some work to do.

src/de/tech-stack.md Outdated Show resolved Hide resolved
@DerZade

This comment was marked as resolved.

@y0014984
Copy link
Contributor

y0014984 commented Nov 28, 2024

Looks better now with round avatars.

image

@nomisum
Copy link
Contributor Author

nomisum commented Nov 28, 2024

Fonts are still different between regular text and list.

Screenshot 2024-11-28 105012

image

cant reproduce

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.

3 participants