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

feat: optimze ui in dark mode, add color themes, replace dark mode impl, add debug inspector #31

Merged
merged 8 commits into from
Jun 3, 2024

Conversation

Innei
Copy link
Member

@Innei Innei commented Jun 3, 2024

Signed-off-by: Innei i@innei.in

Main Changes

  1. Insert theme detect script to avoid page first fresh.
  2. Init tailwind color palette
  3. Fix some wrong color in dark mode
  4. Unify header button UI style.

For DX

  1. Add click-to-component to quick jump to code location from UI

Innei added 2 commits June 3, 2024 14:43
Signed-off-by: Innei <i@innei.in>
Signed-off-by: Innei <i@innei.in>
@Innei
Copy link
Member Author

Innei commented Jun 3, 2024

CleanShot 2024-06-03 at 4  05 06@2x
CleanShot 2024-06-03 at 4  05 18@2x


CleanShot 2024-06-03 at 4  05 37@2x
CleanShot 2024-06-03 at 4  06 13@2x

Signed-off-by: Innei <i@innei.in>
@hyoban
Copy link
Member

hyoban commented Jun 3, 2024

jotai-dark has disableTransition option too 🥹

https://github.com/hyoban/jotai-dark#usage

@Innei Innei requested a review from DIYgod June 3, 2024 08:09
@Innei
Copy link
Member Author

Innei commented Jun 3, 2024

jotai-dark has disableTransition option too 🥹

hyoban/jotai-dark#usage

After customizing atomDark's configuration in the case of multiple windows, there may be issues in multiple windows

@Innei
Copy link
Member Author

Innei commented Jun 3, 2024

@hyoban Maybe there are some bugs in the color transition.

CleanShot.2024-06-03.at.4.29.06.mp4

@Innei
Copy link
Member Author

Innei commented Jun 3, 2024

@hyoban This way is possible, maybe there is a problem with the css injection script provided by jotai-dark


const isDarkAtom = atomDark({
  // disableTransition: true,

  storageKey: "theme",
  applyDarkMode(isDark) {
    const $css = document.createElement("style");
    $css.innerHTML = `* {
      transition: none !important;
    }`;
    document.head.appendChild($css);

    void document.body.offsetHeight;

    document.documentElement.dataset.theme = isDark ? "dark" : "light";

    void document.body.offsetHeight;
    $css.remove();
  },
});

Innei added 2 commits June 3, 2024 16:37
Signed-off-by: Innei <i@innei.in>
Signed-off-by: Innei <i@innei.in>
src/renderer/src/router.tsx Outdated Show resolved Hide resolved
Signed-off-by: Innei <i@innei.in>
<HeaderActionButton
tooltip={unreadOnly ? "Unread Only" : "All"}
onClick={() => setUnreadOnly(!unreadOnly)}
active={unreadOnly}
Copy link
Member

@DIYgod DIYgod Jun 3, 2024

Choose a reason for hiding this comment

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

The active status may no longer be necessary since the icon already indicates it. Additionally, the style of the active state feels somewhat discordant, I preferred the previous rounded

Copy link
Member

Choose a reason for hiding this comment

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

Also, I feel like this button is a bit big, what do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wonder if it would be better to unify the style of HeaderActionButton.

@@ -27,7 +24,6 @@ export function Component() {
<div
className={cn(
"h-full shrink-0 overflow-y-auto border-r",
window.electron ? "pt-10" : "pt-4",
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 it's too tight here, it's better to keep a little padding

Copy link
Member Author

Choose a reason for hiding this comment

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

Would it be visually better to have the HeaderActionButton on the same baseline

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure, but you can try to improve it. I can merge this PR first since it includes other modifications.

hyoban added a commit to hyoban/jotai-dark that referenced this pull request Jun 3, 2024
return (
<div className="flex items-center justify-between gap-4">
{/* TODO system color mode */}
Copy link
Member

Choose a reason for hiding this comment

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

Just a note. Currently, jotai-dark will reset back to the system theme if user preference is the same as the system.

ScreenShot 2024-06-03 18 35 28

Signed-off-by: Innei <i@innei.in>
@DIYgod DIYgod merged commit 3178c06 into main Jun 3, 2024
1 check passed
@DIYgod DIYgod deleted the chore/ui-optimize branch June 3, 2024 10:50
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