Feat: Autoplay pages in fullscreen #35981
Conversation
💚 Build Succeeded |
💚 Build Succeeded |
💚 Build Succeeded |
w33ble
added
Team:Presentation
|
Pinging @elastic/kibana-canvas |
💚 Build Succeeded |
|
Works like a charm, there is one thing I noticed though. We're missing a tooltip on hover / key for these settings. As long as this is fixed, LGTM
I just added @shaunmcgough as a reviewer as well. |
|
Added this, happy to change the text or whatever.
|
💔 Build Failed |
💚 Build Succeeded |
|
@w33ble I have a small PR coming with some minor layout touch up. |
There was a problem hiding this comment.
This looks great! There are a couple small layout changes that I've placed in a PR:
w33ble#9
- Keeps the custom interval forms aligned at bottom of panel
- Converts the 'disable auto-refresh' from a link to an icon button and places next to the manual refresh icon button
- Change the wording of the cycle section to more closely match the refresh seciton
The after state will now look like:
There was a problem hiding this comment.
Nice... easy to follow.
As a team, we've got to got to get into the habit of 1/ writing new code in Typescript, 2/ ensuring our components are not monolithic and 3/ backing our components with stories (which automatically create tests). I would strongly prefer you commit this with those considerations in mind.
From the look of things, this is a prime candidate for these priorities... the types are simple, the stories should be easy to write, and the strong types will help detect any bugs or assumptions in the logic.
| refreshInput = null; | ||
| return ( | ||
| <div> | ||
| <EuiFlexGroup alignItems="center" justifyContent="spaceAround" gutterSize="xs"> |
There was a problem hiding this comment.
This component has a ton of moving parts and a lot of markup. I'd like to see this component broken up into its disparate parts as separate components with independent stories/snapshots... and Typescript'd. This component in particular should be fairly straightforward.
x-pack/plugins/canvas/public/components/workpad_header/control_settings/custom_interval.js
Outdated
Show resolved
Hide resolved
x-pack/plugins/canvas/public/components/workpad_header/control_settings/index.js
Outdated
Show resolved
Hide resolved
| import { timeDurationString } from '../../../lib/time_duration'; | ||
| import { CustomInterval } from './custom_interval'; | ||
|
|
||
| const ListGroup = ({ children }) => <ul style={{ listStyle: 'none', margin: 0 }}>{[children]}</ul>; |
There was a problem hiding this comment.
It's interesting you have RefreshItem in scope and this as global. I'd relocate the other here to avoid re-renders.
There was a problem hiding this comment.
RefreshItem uses a prop. I could hoist it via a partial, but either way the location of the helper shouldn't affect re-renders as far as I know. What am I missing?
| <EuiFlexGrid gutterSize="s" columns={2}> | ||
| <EuiFlexItem> | ||
| <ListGroup> | ||
| <RefreshItem duration="5000" label="5 seconds" /> |
There was a problem hiding this comment.
Suggestion: make this functional, e.g. create a number[] constant and write a function that takes a duration and produces the item with the appropriate label. That will make these ListGroups more terse, and allows us to add/remove durations at will.
There was a problem hiding this comment.
I don't understand how you're suggesting I change this.
x-pack/plugins/canvas/public/components/workpad_header/fullscreen_control/fullscreen_control.js
Show resolved
Hide resolved
|
Noticed this weird interaction that I don't think is the intended result. When you select a cycle interval, the Refresh Control receives focus and it's tool tip appears. Same thing also happens when you click "Disable Auto Refresh" I poked around for a minute and couldn't seem to figure out what's causing it to receive the focus. Perhaps it's something in Eui that's making it happen.
|
|
@crob611 that is weird, and certainly not intended. I see it too... but I don't see any code that controls focus, so I suspect this is either EUI weirdness or maybe there's some more component wrapping I need to add. @ryankeairns you have any idea why this button focusing happens? |
|
@ryankeairns merged (or rather, cherry-picked) your changes. Mind taking another look? |
💚 Build Succeeded |
💚 Build Succeeded |
|
YESSSS 🎉 🎉 🎉 🎉 🎉 🎉 🎉 nice work @w33ble !!! :alex_approved: |
* feat: add autoplay redux boilerplate WIP auto-play settings * feat: add page cycle settings * feat: add cycle toggle hotkey * chore: add tooltip text to settings icon * settings layout * fix: handle invalid input for custom interval * chore: address nit
* feat: add autoplay redux boilerplate WIP auto-play settings * feat: add page cycle settings * feat: add cycle toggle hotkey * chore: add tooltip text to settings icon * settings layout * fix: handle invalid input for custom interval * chore: address nit
* feat: add autoplay redux boilerplate WIP auto-play settings * feat: add page cycle settings * feat: add cycle toggle hotkey * chore: add tooltip text to settings icon * settings layout * fix: handle invalid input for custom interval * chore: address nit
* feat: add autoplay redux boilerplate WIP auto-play settings * feat: add page cycle settings * feat: add cycle toggle hotkey * chore: add tooltip text to settings icon * settings layout * fix: handle invalid input for custom interval * chore: address nit
Summary
Part of #23061
Adds new settings and functionality that will auto-cycle through the pages in a workpad when in fullscreen mode, on a set interval.
Pages will only rotate in fullscreen mode, and only automatically if the toggle is set. When leaving fullscreen mode, the playing stops, and it will resume when back in fullscreen mode, if the feature is still enabled (the play state is preserved).
And while in fullscreen mode, users can use the
photkey to toggle autoplay of pages.How it works
pwhen in fullscreen mode. It is disabled by default.p(to disable the feature), or by exiting fullscreen mode. The state is preserved, so if it's playing and you exit fullscreen, it will start playing again once you enter fullscreen again. Likewise, if you toggle it off and leave fullscreen mode, it will remain off when you go back into it.