Review of "TouchUp" version 1.6.0 (20)

Details Page Preview

Enhances various aspects of the GNOME experience when using a touch device. Current features: - Navigation bar (gestures and buttons mode), as known from Android (supports Dash to Dock and Dash to Panel) - Touch gestures for notifications (scroll, dismiss, collapse, expand, hide banner) - Swipe down the panel to smoothly open any menu, swipe up open menus to dismiss - Onscreen Keyboard: key popups, extended keys, swipe to close, quick paste button, switch layouts by space bar swiping - Manually open the keyboard by swiping up the bottom left navigation bar edge - Floating rotate button when auto-rotation is off, as known from Android - Overview and desktop background gestures (switch workspaces, open/close overview) - Overview window preview gestures (swipe up to close windows, hold+move to drag) - Double tap panel, desktop background or lockscreen to lock & sleep A lot more is planned; take a look at the roadmap for that: https://github.com/mityax/gnome-extension-touchup Notice: If you enable the OSK quick paste action feature, TouchUp will access your clipboard – its content, however, never leaves your devices memory (i.e. is not written to a file, or sent via network). If you find this extension valuable and can spare some funds, please donate to make sure the work can go on! :)

Extension Homepage
https://github.com/mityax/gnome-extension-touchup

No comments.

Diff Against

Files

Note: Binary files aren't shown on the web site. To see all files, please download the extension zipfile.

Shexli (experimental) warning 2 manual_review 1

Shexli found 3 issues that may need reviewer attention.

EGO-A-005 manual_review

extensions should not access the clipboard directly

Direct clipboard access via `St.Clipboard.get_default()` requires reviewer scrutiny.

Review Guidelines

  • features/osk/_oskQuickPasteActionFeature.js:91
    St.Clipboard.get_default()
  • features/osk/_oskQuickPasteActionFeature.js:138
    St.Clipboard.get_default()

EGO-L-004 warning

main loop sources should be removed in disable()

Main loop sources assigned in `enable()` are missing matching removals in `disable()` or its helper methods.

Remove main loop sources

  • features/navigationBar/widgets/gestureNavigationBar.js:142
                GLib.idle_add(GLib.PRIORITY_DEFAULT_IDLE, () => {
                    if (!this.styleClassUpdateInterval.enabled) // if the interval/extension is disabled, cancel idle directly
                        return GLib.SOURCE_REMOVE;
                    let rect = this.pill.get_transformed_extents();
        

EGO-M-008 warning

extensions using unlock-dialog must document it in disable() comments

Extensions using `unlock-dialog` should document the reason in `disable()` comments.

Session Modes

  • extension.js
    unlock-dialog declared but disable() was not found

All Versions

Version Status
1.6.0 (20) Active
1.5.0 (19) Active
1.5.0 (18) Rejected
1.5.0 (17) Rejected
1.4.0 (16) Active
1.4.0 (15) Rejected
1.3.5 (14) Active
1.3.4 (13) Active
1.3.3 (12) Active
1.3.2 (11) Active
1.3.1 (10) Active
1.3.0 (9) Active
1.3.0 (8) Rejected
1.2.2 (7) Active
1.2.1 (6) Rejected
1.2.0 (5) Active
1.1.0 (4) Active
3 Active
2 Rejected
1 Rejected

Previous Reviews on this Version

JustPerfection rejected
1. Why are you creating functions like that in a class (line 154-155 `extension.js`)? 2. You shouldn't manage another extension (line 66 `features/integrations/dashToDockIntegrationFeature.js`).
mityax posted a review
Thx for the review. 1. This simplifies & hardens stuff a lot with typescript, since these functions are just redirecting and can inherit the type-safe mapping of signal names to handler signatures. This is not possible to express in TS without property functions. Is it okay this way or do I should I change this? 2. I think this is the only way to access the dockManager, it's explicitly exported by Dash2Dock for this purpose (see below) and is not exposed as a class attribute. I made sure to only import the module if the extension is enabled (it's in ESM module cache then anyways, so this is side-effect free). Would you know of another way I can access the dockManager? Thanks for your feedback! This is the Dash2Dock extension.js for reference: ``` import {DockManager} from './docking.js'; import {Extension} from './dependencies/shell/extensions/extension.js'; // We export this so it can be accessed by other extensions export let dockManager; export default class DashToDockExtension extends Extension.Extension { enable() { dockManager = new DockManager(this); } disable() { dockManager?.destroy(); dockManager = null; } } ```
JustPerfection active
Ok. Approved. Also no need to change the #1 if that's too much work.
mityax posted a review
Thanks you! Also, it's good to have you back on here the whole community was really missing you, I think :)