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

Basic palette selector #25

Closed
wants to merge 1 commit into from
Closed

Basic palette selector #25

wants to merge 1 commit into from

Conversation

Cveinnt
Copy link
Contributor

@Cveinnt Cveinnt commented Nov 6, 2023

Description

This PR implements a basic palette selector in the form of a dropdown menu, and added some basic themes for the users to choose from. The preset themes are converted from https://github.com/alacritty/alacritty-theme/.

Dropdown menu preview:

Screenshot

Example theme preview:

Screenshot

Copy link
Owner

@ekzhang ekzhang left a comment

Choose a reason for hiding this comment

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

Looks good, thanks so much for doing this! I'll have to check it out before we merge, but probably works great :)

@@ -68,6 +66,16 @@
export let termEl: HTMLDivElement = null as any; // suppress "missing prop" warning
let term: Terminal | null = null;

import { settings } from "$lib/settings";
import type { ITheme } from "xterm";
Copy link
Owner

Choose a reason for hiding this comment

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

We should move imports to the top of the file and sort them

<div class="text-red-500">Coming soon</div>
<div>
<select
class="w-52 px-3 py-1.5 rounded-md bg-zinc-700 outline-none focus:ring-2 focus:ring-indigo-500"
Copy link
Owner

Choose a reason for hiding this comment

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

From your screenshot, the arrow at the right side of the dropdown (when closed) is a bit too close to the right edge of the element

let theme: ITheme = themes.defaultDark;

$: if ($settings.theme && term) {
theme = themes[$settings.theme];
Copy link
Owner

Choose a reason for hiding this comment

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

Can we fall back to defaultDark if this is undefined?

@ekzhang
Copy link
Owner

ekzhang commented Jan 2, 2024

Hi @Cveinnt, any update on this?

@ekzhang
Copy link
Owner

ekzhang commented Apr 6, 2024

Hi @Cveinnt — I haven't gotten a response in 3 months, hope all is well. I'll use this as the base of implementing the feature 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