Skip to content

Change the design#415

Open
PorametPat wants to merge 14 commits intomainfrom
shadcn-ui
Open

Change the design#415
PorametPat wants to merge 14 commits intomainfrom
shadcn-ui

Conversation

@PorametPat
Copy link
Contributor

TD;LR

Use shadcn-ui instead of daisy-ui.

What changes

Why shadcn-ui

  • Provides a better looking UI components while allows for fully customization.
  • Using schematic color instead of predefined theme.

What changes

  • Improve Navigation bar for large screen and use sidebar for mobile screen.
  • In the members page, organize member cards using grid instead. Improve the design of member card and use Dialog instead of Accordion for further details.

Additional information.

Copy link
Member

@auspicacious auspicacious left a comment

Choose a reason for hiding this comment

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

@PorametPat

  • Why does this PR break Sara's image?
  • Why does this PR add SDD files? What are those?
  • Looking at this in my browser, the first thing I notice is that the new design is more visually distracting than the old one. Rather than the the menu bar being solid and extending all the way to the top of the frame, it's semi-transparent and has a gap between the top of the frame and the menu section.

This causes visual confusion and distraction, makes the menu harder to read, and makes it much easier to accidentally click on something behind the menu bar rather than on the menu bar itself. Simpler is better. Human beings have only a very limited capacity for visual complexity.

image

{
label: "Books",
href: "/publications#books",
// description: "Cillum duis deserunt esse ipsum ex enim cillum labore dolore. Sit anim et nisi aliqua dolor duis commodo ullamco aute laboris est fugiat ea aliquip."
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image
This is the behavior that I get in my browser (chromium based). My intention is to implement glass-like effect not just semi-transparent. But since the behavior is not consistent, I will change it to a solid style at the top as you suggest.

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 SDD files are already there in the main branch. I'm not sure why it change in my branch either. I will check sara-senpai's photo.

Copy link
Member

Choose a reason for hiding this comment

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

@PorametPat

This is the behavior that I get in my browser (chromium based).

In Firefox it looks similar, but when Firefox takes a screenshot it appears more transparent. However, both are highly problematic. Having a constantly changing, unpredictably colored background harms readability and makes it harder to identify the menu.

And, as I mentioned, having a gap between the top of the frame and the top of the menu means that I could accidentally click on something in that gap when I mean to click on the menu.

Basically, this layout creates unnecessary accessibility problems.

@PorametPat
Copy link
Contributor Author

There are problems with navigation bar when using keyboard navigation. Also the package rely on next 15 (not stable) and react 19 which break the web when I upgrade.

@auspicacious
Copy link
Member

@PorametPat I'm not sure what your last comment is supposed to mean.

@PorametPat
Copy link
Contributor Author

I'm sorry. I've noticed the problem with keyboard navigation. So I will fix them first. I'm also considering upgrading the framework version, but that attempt failed. And something coming up, so I didn't working on this now.

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