Repository navigation
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⛔ Files ignored due to path filters (1)
⚙️ Run configuration
⛔ Files ignored due to path filters (1)
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThis change adds surface model metadata and geometry resolution, then integrates surface selection, rendering, feedback, and per-surface zoom into the Buttons view. It also adds tests and user-guide coverage. ChangesSurface-aware Button Grid
Priority: ⬇️ Low
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check | Issue #2189 asks for the Buttons view to reflect an attached Stream Deck +, for rotary encoder options to be enabled by default, and for manual layout selection when no Stream Deck is attached. This P… |
Please add automatic selection of the attached Stream Deck + and enable its rotary encoder options by default, with tests for both behaviors. The manual selection feature already addresses the no-device selection requirement. |
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Out of Scope Changes check | ✅ Passed | The surface model IPC, layout resolution, canvas rendering, zoom, dropdown support, tests, and Buttons guide all support the surface-view feature requested by issue #2189. The change summary identifie… |
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly and concisely describes the main change: adding a surface view for the button grid. |
Full details: Linked Issues check
Explanation
Issue #2189 asks for the Buttons view to reflect an attached Stream Deck +, for rotary encoder options to be enabled by default, and for manual layout selection when no Stream Deck is attached. This PR supports manual selection through GridViewAsControl and useGridViewAs. However, the new view state defaults to disabled with no selection, so the PR does not establish automatic selection of an attached Stream Deck +. The change summary also does not show implementation or tests for enabling rotary encoder options by default.
- Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A face takes shape in measured space,
Controls find their positions and place.
LEDs trace a ring or strip,
Zoom fits the panel with a flip.
Models join the grid view flow,
While colors make the buttons glow.
Tests chart each change as features grow.
Comment @coderabbitai help to get the list of available commands.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
So is it just that required to do the "col span" type behaviour on the Neo to get the "long" button at the bottom to align with the four normal buttons above it, or is there some additional work/future enhancement required first? I assume the pain of stuff like the Shuttle with a row of four buttons, then a row of five below will always be a pain/you won't necessarily support different column spacing (I guess like the SD + XL), although in theory with some clever col spans, it ought to be possible to represent. |
|
The current screenshots show the basic 'auto' mode of displaying a surface. Our best guess at how to arrange things with the limited information provided. The part I am still working on is detailed drawing, relying on the surface module providing more details. This is currently achieving the below but it needs some more work (Largely background images, and figuring out some edge cases): |
That already looks pretty awesome for those ones.
Are those edge cases likely to go as far as the complexities of Shuttle which is effectively a doughnut and then the whole as separate controls in terms of the jog shuttle? |
d8a91b5 to
abf4777
Compare
e2e8c4a to
fba3ccd
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
dd7612af-9b83-4b27-afc6-14d7cb665afb
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (66)
companion/lib/Instance/Surface/ChildHandler.tscompanion/lib/Instance/Surface/IpcTypes.tscompanion/lib/Instance/Surface/Thread/Entrypoint.tscompanion/lib/Preview/Graphics.tscompanion/lib/Surface/Config.tscompanion/lib/Surface/Controller.tscompanion/lib/Surface/Handler.tscompanion/lib/Surface/IP/ElgatoEmulator.tscompanion/lib/Surface/IP/Satellite.tscompanion/lib/Surface/LayoutSummary.tscompanion/lib/Surface/PluginPanel.tscompanion/lib/Surface/PluginSurfaceModels.tscompanion/lib/Surface/Types.tscompanion/test/Preview/Graphics.test.tscompanion/test/Surface/ControllerLayouts.test.tscompanion/test/Surface/ControllerModels.test.tscompanion/test/Surface/LayoutSummary.test.tscompanion/test/Surface/PluginSurfaceModels.test.tsdocs/user-guide/3_config/buttons/index.mdpackage.jsonshared-lib/lib/Model/Common.tsshared-lib/lib/Model/Surfaces.tsshared-lib/lib/ModuleApiVersionCheck.tsshared-lib/lib/SurfaceLayout.tsshared-lib/lib/SurfaceLayoutEstimate.tsshared-lib/lib/__tests__/SurfaceLayout.test.tsshared-lib/lib/__tests__/SurfaceLayoutEstimate.test.tswebui/src/Buttons/ButtonGridPanel.csswebui/src/Buttons/ButtonGridPanel.tsxwebui/src/Buttons/ButtonGridZoomControl.stories.tsxwebui/src/Buttons/ButtonGridZoomControl.tsxwebui/src/Buttons/ButtonInfiniteGrid.tsxwebui/src/Buttons/GridButtonCell.tsxwebui/src/Buttons/GridButtonPreview.tsxwebui/src/Buttons/GridViewAs.csswebui/src/Buttons/GridViewAs.tswebui/src/Buttons/GridViewAsBanner.stories.tsxwebui/src/Buttons/GridViewAsBanner.tsxwebui/src/Buttons/GridViewAsControl.tsxwebui/src/Buttons/GridViewGrowPanel.tsxwebui/src/Buttons/GridZoom.tsxwebui/src/Buttons/SurfaceView/SurfaceCanvas.csswebui/src/Buttons/SurfaceView/SurfaceCanvas.tsxwebui/src/Buttons/SurfaceView/SurfaceLeds.csswebui/src/Buttons/SurfaceView/SurfaceLeds.tsxwebui/src/Buttons/SurfaceView/__tests__/ledLayout.test.tswebui/src/Buttons/SurfaceView/__tests__/surfaceGeometry.test.tswebui/src/Buttons/SurfaceView/ledLayout.tswebui/src/Buttons/SurfaceView/surfaceGeometry.tswebui/src/Buttons/__tests__/ButtonGridPanel.test.tsxwebui/src/Buttons/__tests__/ButtonsPage.test.tsxwebui/src/Buttons/__tests__/GridButtonDragOverlay.test.tsxwebui/src/Buttons/__tests__/GridButtonIcons.test.tsxwebui/src/Buttons/__tests__/GridButtonPreview.test.tsxwebui/src/Buttons/__tests__/GridViewAs.test.tswebui/src/Buttons/__tests__/GridZoom.test.tsxwebui/src/Buttons/index.tsxwebui/src/Buttons/useGridViewAs.tswebui/src/Components/DropdownChoices.tsxwebui/src/Components/DropdownInputField/Popup.tsxwebui/src/Components/DropdownInputField/useFuzzyChoices.tswebui/src/Components/DropdownInputFieldSimple.tsxwebui/src/Components/__tests__/DropdownInputField.test.tsxwebui/src/Hooks/__tests__/useButtonImageForLocation.test.tsxwebui/src/Hooks/useButtonImageForLocation.tswebui/src/ImportExport/Import/Page.tsx
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.
|
I am happy with this now, and think it is ready to merge and update other modules to properly support. @peternewman check out the last 3 screenshots in the description. Thats what I've been aiming for, and will hopefully translate to other surface modules just fine. |
fba3ccd to
a775b9b
Compare
|
This looks great; nice work! |
a775b9b to
79ee3b9
Compare
A choice can say it is disabled, and both the simple and the searchable dropdown then show it without letting it be chosen. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The surface api for declaring the models a plugin supports, describing the appearance of a surface, and saying which declared model a surface is, is not yet in a release. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The grid is larger than any one surface, and a surface is not always a plain rectangle of square buttons. Viewing the grid as a surface - one which has been connected, or a model of one a surface module declares - draws the grid as the face of that surface: each control where it sits and at the shape it is drawn, with the cell coordinates still the grid's own. Controls beyond the configured grid are drawn but cannot be configured, and offer to grow the grid to fit. The choice is made per browser, as a way of looking at the grid rather than part of the config. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A surface can describe its face: where each control sits and its shape, the colour of its body, and artwork for it. That is kept with the surface's config alongside its layout, so it is known while the surface is unplugged, and the view draws it exactly rather than estimating positions from the layout's rows and columns. Artwork is drawn without the body colour under it, so the shape of the device shows, and a control is only rounded when given a radius. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…in ones A fixed few pixels taken off every side of a control was nothing on a key but squashed a thin one, like the Neo's info bar, out of the shape its image is drawn at. Controls are now drawn exactly where the surface has them, as the face already leaves gaps between them. A control thinner than a fingertip is given a hit area grown around it, cut back to the middle of the gap wherever it would meet another's, so even a dense face never has two overlap. One too small to hold its location leaves it out. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A style with colours and no bitmap, like the Neo's touch buttons, is lit rather than drawn on. Such a control is drawn as a swatch of the button's background, which the preview stream now carries, rather than its image squeezed into a sliver it was never going to be shown in. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A control's style can declare leds, as a ring around an encoder or a strip. The preview stream now carries the gauge a button drives them with, and the view draws them around or along the control, each segment lit with the same sampling the surface is driven with. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The dark ground the grid uses fights with a face of any colour but its own, so a face the surface describes is drawn on the page instead, with a hairline and soft shadow tracing its outline, and scrollbars to match. Press mode keeps the armed ground, as marking out what is live matters more there. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The panel's min-height was only ever set by the grid, so a surface face kept whatever the grid had last asked for, and zooming out left the panel far taller than the face. The face now asks for its own: the whole face when it is short, or two rows of keys when it is tall, so it scrolls within the panel rather than stretching the page. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…by default A Neo and a Studio want very different zoom levels, so one shared between them is always wrong for one. Viewing as a surface has a zoom of its own, remembered for its model so a plugged in surface shares it with its model. One not zoomed by hand fits to the panel's width, following it as it changes, within 60-100% so a very wide surface stays readable and a small one is not blown up. The zoom control offers going back to fitting. Steps work from the previous setting, so a burst of them - a fast ctrl+wheel - all count, and the controller only changes with the kind of surface, so the wheel listener holding it is not redone at every step. The grid's own zoom now starts at 100% when storage cannot be read, rather than failing the buttons page where storage is blocked. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A surface can say which of its module's declared models it is. That is kept with its config, like its appearance, so it is still known while the surface is unplugged, and the layout summary qualifies it with the module, as the declared models are. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Matching by the model id a surface reports, before falling back to its name, which a module is free to word differently from the model's. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A display bigger than the others in its row or column, like an info bar under a row of keys, now spills into the empty cells beside it rather than widening its own track, so the keys around it stay evenly spaced. A cell two would both spill into is left to neither. A control with no display fills the cell the displays around it leave, rather than being drawn a fixed square, while one with a ring of leds stays round. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
79ee3b9 to
13352bf
Compare
This branch has not been deployed
3 participants
Related: #1352
Closes: #2189
closes #3205
Replaces #2945
This is intended to solve 2 workflow problems:
x-xx-xx-xpattern of encoder/lcd alignment.For now this is gated behind a flag in the advanced settings page, so that this can be merged before being complete (to avoid large merge conflicts)
To achieve this, there is a new button+dropdown in the button grid, which allows for choosing a surface to see the grid as. This has 2 modes, you can pick a surface that your companion currently or has previously had connected (once it has been connected recently enough for us to grab and persist the layout info), or any model reported as compatible by an enabled surface module.
The grid below then gets reconfigured to match the selected surface. For modules which aren't providing a new 'surfaceAppearance' definition, we make our best attempt based upon the technical layout of the controls it requests. These are usually messy, but at least filter down the controls and provide the correct resolutions to the controls: (Some CSS polish still needed)
Surfaces can now also provide a full appearance description to drive this drawing properly. For example:
Note that these are not 100% faithful to the original, but they should be accurate enough for any view and alignment a user may want to do. Primarily as they are svgs measured from either photos or manufacturer pngs. Intentionally using svgs here to avoid the bloat of high res pngs which will need to come in over satellite.
Summary by CodeRabbit