Skip to content

feat(gui-client): set a different tray icon when signed out#5817

Merged
ReactorScram merged 39 commits into
mainfrom
feat/tauri-set-icon
Jul 11, 2024
Merged

feat(gui-client): set a different tray icon when signed out#5817
ReactorScram merged 39 commits into
mainfrom
feat/tauri-set-icon

Conversation

@ReactorScram
Copy link
Copy Markdown
Contributor

@ReactorScram ReactorScram commented Jul 9, 2024

Closes #5810

### Tasks
- [x] Try not to set the icon every time we change Resources
- [x] Get production icons
- [x] Add changelog comment
- [x] Add CI stress test that sets the icon 10,000 times
- [x] Open for review
- [x] Repair changelog
- [x] Merge

@ReactorScram ReactorScram self-assigned this Jul 9, 2024
@vercel
Copy link
Copy Markdown

vercel Bot commented Jul 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
firezone ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 11, 2024 7:07pm

Copy link
Copy Markdown
Contributor Author

@ReactorScram ReactorScram Jul 9, 2024

Choose a reason for hiding this comment

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

@jamilbk You can replace this file, I'm planning to package this in the binary and extract it to a cache dir the same way we unpack wintun.dll. It ends up writing redundant data to disk but it looks easy and it's only 20 KB this is the placeholder signed-out icon

Also 2 other questions:

  1. Do we need a "signing in" icon in addition to "signed in" and "signed out"? There are actually 5 possible menu states, the other 3 are "Waiting on browser", "Waiting on connlib", and "Loading" https://github.com/firezone/firezone/pull/5817/files#diff-f99d43b72eb35bc908c93849dd50e0b2be5e2c676b72e4f9266f1c3caeca9deaR57-R62
  2. While we're fixing the icons, can we fix bug(gui-client): the tray icon has poor contrast against the default dark Ubuntu task bar #5014? It came up again with Fix favicon #5822

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 9, 2024

Terraform Cloud Plan Output

Plan: 15 to add, 23 to change, 15 to destroy.

Terraform Cloud Plan

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 9, 2024

Performance Test Results

TCP

Test Name Received/s Sent/s Retransmits
direct-tcp-client2server 230.9 MiB (-1%) 232.4 MiB (-1%) 259 (+67%)
direct-tcp-server2client 241.1 MiB (+1%) 242.8 MiB (+1%) 484 (+122%)
relayed-tcp-client2server 233.8 MiB (-3%) 234.5 MiB (-3%) 378 (+1%)
relayed-tcp-server2client 238.1 MiB (+0%) 238.8 MiB (+0%) 504 (-24%)

UDP

Test Name Total/s Jitter Lost
direct-udp-client2server 500.0 MiB (-0%) 0.03ms (+44%) 45.44% (+18%)
direct-udp-server2client 500.0 MiB (-0%) 0.01ms (+3%) 22.25% (-8%)
relayed-udp-client2server 500.0 MiB (-0%) 0.04ms (+88%) 55.57% (+5%)
relayed-udp-server2client 500.0 MiB (-0%) 0.03ms (+349%) 33.69% (-5%)

@ReactorScram ReactorScram changed the base branch from main to refactor/gui-simplify-resource-updating July 10, 2024 16:59
Ok(())
}

fn tray_menu() -> Result<()> {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Wasn't needed anymore

Base automatically changed from refactor/gui-simplify-resource-updating to main July 10, 2024 21:49
@ReactorScram ReactorScram changed the base branch from main to chore/changelog-platform-specific July 10, 2024 22:15
…icon

Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>
@ReactorScram ReactorScram marked this pull request as ready for review July 10, 2024 22:25
run: pnpm tailwindcss -i src/input.css -o src/output.css
- name: Build client
run: cargo build -p firezone-gui-client
run: cargo build -p firezone-gui-client --all-targets
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This allows us to remove cargo build invocations from the gui-smoke-test Rust binary

@ReactorScram ReactorScram requested a review from conectado July 10, 2024 22:34
Base automatically changed from chore/changelog-platform-specific to main July 11, 2024 16:12
Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>
Comment thread website/src/components/Changelog/GUI.tsx
Copy link
Copy Markdown
Contributor

@conectado conectado left a comment

Choose a reason for hiding this comment

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

LGTM!

Left a few comments but nothing pressing/blocking, feel free to merge

pub(crate) enum Cmd {
SetAutostart(SetAutostartArgs),
TrayMenu,
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we not have a subdommand now?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was for debugging a previous PR and I didn't need it anymore

.await
.context("Failed to send ClearLogs request")?;

// Tray icon stress test
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what's the purpose of this stress test?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There is a strange part deep inside Tauri that exports the icon as a PNG to /run every time we call set_icon: https://github.com/tauri-apps/tao/blob/tao-v0.16.7/src/platform_impl/linux/system_tray.rs#L119

I wanted to make sure this doesn't take too long or use too much disk. It looks fine, and /run is supposed to be a RAM disk, so it won't use real disk space. But in case there's a performance regression or something, it could show up here.

// TODO: Show some "Waiting for portal..." state if we got the deep link but
// haven't got `on_tunnel_ready` yet.
if let Some(auth_session) = self.auth.session() {
let menu = if let Some(auth_session) = self.auth.session() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can reduce indentation a bit by using a let Some(auth_session) = self.auth.session() else { ... } here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm having trouble seeing it because there's 3 branches, it returns menu and then calls self.tray.update

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.

ux(gui-client): Add signed in / signed out icon states to menubar icon

2 participants