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.

Sign up for GitHub

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

Jump to bottom

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
Merged

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

merged 8 commits into from
Jun 3, 2024

Conversation

Innei
Copy link
Member

@Innei Innei commented Jun 3, 2024 •
edited
Loading

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
@Innei
chore: unify header action button style
39a1037 
Signed-off-by: Innei <i@innei.in>
@Innei
fix: dark mode
e158c07 
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

@Innei
chore: fix lockfile
50de71d 
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
@Innei
fix: page color fresh
549667f 
Signed-off-by: Innei <i@innei.in>
@Innei
fix: type error
a202ff7 
Signed-off-by: Innei <i@innei.in>
DIYgod
DIYgod requested changes Jun 3, 2024
View reviewed changes
src/renderer/src/router.tsx Outdated Show resolved Hide resolved
@Innei
fix: type error
2f49f12 
Signed-off-by: Innei <i@innei.in>
DIYgod
DIYgod reviewed Jun 3, 2024
View reviewed changes
src/renderer/src/components/entry-column/index.tsx
<HeaderActionButton
tooltip={unreadOnly ? "Unread Only" : "All"}
onClick={() => setUnreadOnly(!unreadOnly)}
active={unreadOnly}
Copy link
Member

@DIYgod DIYgod Jun 3, 2024 •
edited
Loading

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.

src/renderer/src/pages/(main)/(context)/index.tsx
@@ -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
@hyoban
fix: apply disableTransition
d334a6c 
RSSNext/Follow#31
hyoban
hyoban reviewed Jun 3, 2024
View reviewed changes
src/renderer/src/components/appearance-switch.tsx Outdated
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

@Innei
chore: remove todo
f650ea5 
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
@lantex lantex mentioned this pull request Dec 1, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hyoban hyoban hyoban left review comments

@DIYgod DIYgod DIYgod approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Innei @hyoban @DIYgod