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

feature: providing a minimal launcher written in flutter for ubuntu frame #191

Merged
merged 20 commits into from
Jul 26, 2024

Conversation

mattkae
Copy link
Contributor

@mattkae mattkae commented Jun 24, 2024

What's new?

  • Frame can now optionally support the launcher, which is a minimal dock allowing the user to switch between fullscreen application

Documentation

Ubuntu Frame's launcher daemon provides users with a way to navigate between multiple applications running on a single frame instance.

Enablement

To enable the launcher, run:

snap set ubuntu-frame launcher=true

Behavior

The following specifies the behavior that you can of the launcher daemon:

  • When frame starts, the launcher will appear in the left-hand side of the screen with a black background
  • As window are opened, the launcher will display an icon that corresponds with the application for that window
    • Icons will ONLY be displayed for applications that are packaged as snaps. The desktop files for these applications are found in the /var/lib/snapd/desktop directory.
  • When an icon is clicked, the corresponding window will be brought into focus
  • If a window is closed, then the icon will be removed from the launcher
  • If we cannot resolve an icon for a window, then the launcher will attempt to use the application-x-executable icon. If that fails, then the icon will be missing entirely.

@mattkae mattkae requested a review from Saviq June 24, 2024 12:47
@mattkae mattkae requested a review from a team as a code owner June 24, 2024 12:47
@Saviq
Copy link
Collaborator

Saviq commented Jun 24, 2024

@mattkae can you please draft a documentation page for this? Particularly on what are the requirements for an icon to show up.

Just pop it in here and we'll copy to Discourse Docs when ready (or just publish on DD already with a "Work in progress" note).

Copy link
Collaborator

@Saviq Saviq left a comment

Choose a reason for hiding this comment

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

Handful of small tweaks.

README.md Show resolved Hide resolved
launcher/README.md Outdated Show resolved Hide resolved
snap/hooks/configure Outdated Show resolved Hide resolved
Copy link
Collaborator

@Saviq Saviq left a comment

Choose a reason for hiding this comment

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

Oh and you'll need this to please the review tools:

diff --git a/snap/local/plugs.json b/snap/local/plugs.json
new file mode 100644
index 0000000..426efa7
--- /dev/null
+++ b/snap/local/plugs.json
@@ -0,0 +1,5 @@
+{
+  "desktop-launch":{
+    "allow-installation": "true"
+  }
+}

@mattkae
Copy link
Contributor Author

mattkae commented Jun 24, 2024

Oh and you'll need this to please the review tools:

https://github.com/canonical/ubuntu-frame/actions/runs/9646782833/job/26604055393?pr=191 I am still seeing an error. Did I do it right? Just populated snap/local/plugs.json with:

{
  "desktop-launch": {
    "allow-installation": "true"
  }
}

scripts/bin/run-client-wait Outdated Show resolved Hide resolved
@Saviq
Copy link
Collaborator

Saviq commented Jun 24, 2024

Please add copyright headers to the new files, too

@mattkae
Copy link
Contributor Author

mattkae commented Jun 24, 2024

@Saviq: I just have the remaining question here about CI being broken. Documentation is in the description.

@mattkae mattkae requested a review from Saviq June 24, 2024 17:34
@Saviq
Copy link
Collaborator

Saviq commented Jun 24, 2024

Hmm Send button failed. We need the store ACK for this to pass now.

Copy link
Collaborator

@Saviq Saviq left a comment

Choose a reason for hiding this comment

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

Tiny tweaks, otherwise good!

(but we need to wait for the store ACK before landing)

snap/hooks/configure Outdated Show resolved Hide resolved
scripts/bin/wayland-launch Show resolved Hide resolved
scripts/bin/wayland-launch Outdated Show resolved Hide resolved
@Saviq
Copy link
Collaborator

Saviq commented Jul 16, 2024

@mattkae we'll need to disable this for armhf, since Flutter doesn't support it.

See https://github.com/canonical/mir-test-tools/blob/7e593d47308ed20608d634b8b2525bb1df12bc2a/snap/snapcraft.yaml#L211-L229 for reference

@mattkae mattkae requested a review from Saviq July 16, 2024 12:36
Comment on lines 116 to 117
if [ $(uname -m) = "arm" ]; then
echo "Cannot enable the launcher on an armhf system"
Copy link
Collaborator

Choose a reason for hiding this comment

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

We also build for risc, so best reverse that. Maybe there's something in the environment we can use?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Reverse as in have an allow-list, not a deny-list

@mattkae mattkae requested a review from Saviq July 16, 2024 13:43
snap/hooks/configure Outdated Show resolved Hide resolved
@mattkae
Copy link
Contributor Author

mattkae commented Jul 16, 2024

@Saviq Do you know what CI is angry about?

remote: Create a merge proposal for 'main' on Launchpad by visiting:
remote:       https://code.launchpad.net/~mir-ci-bot/mir-ci-bot-craft-remote-build/+git/snapcraft-ubuntu-frame-92e4eac9d32b978bbdb01c7959f0fcb9/+ref/main/+register-merge

@Saviq
Copy link
Collaborator

Saviq commented Jul 16, 2024

@Saviq Do you know what CI is angry about?

It just timed out, looks like LP went on hiatus.

The "merge proposal" message is always there, just INFO when pushing to Launchpad git.

Comment on lines 78 to 97
slots:
# Should be a plug but one snap can't have both the wayland slot and plug. This works.
- wayland
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, we should actually reverse this and have a wayplug instead. Some of our scripting relies on the :wayland slot being there - and it's the more important side anyway.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@mattkae so this is the last bit here, I believe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

scripts/bin/wayland-launch Outdated Show resolved Hide resolved
@mattkae mattkae requested a review from Saviq July 18, 2024 13:59
@Saviq Saviq requested a review from AlanGriffiths July 25, 2024 16:32
@Saviq
Copy link
Collaborator

Saviq commented Jul 25, 2024

@AlanGriffiths since I was all over this, would you please have a look here, disregarding the launcher/ folder - just the snap stuff?

Copy link
Collaborator

@Saviq Saviq left a comment

Choose a reason for hiding this comment

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

This all looks good to me, just tweaked some of the snap details.

Copy link
Contributor

@AlanGriffiths AlanGriffiths left a comment

Choose a reason for hiding this comment

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

Looks sensible. Just one nit about disconnecting desktop-launch without disabling the "launcher"

snap/hooks/configure Show resolved Hide resolved
Copy link
Contributor

@AlanGriffiths AlanGriffiths left a comment

Choose a reason for hiding this comment

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

That should do

@AlanGriffiths AlanGriffiths added this pull request to the merge queue Jul 26, 2024
Merged via the queue into main with commit 8fb0621 Jul 26, 2024
3 checks passed
@AlanGriffiths AlanGriffiths deleted the feature/multi-app branch July 26, 2024 10:19
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