-
Notifications
You must be signed in to change notification settings - Fork 4
feat: add shocker logs page #147
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
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a shocker logs page accessible via the route /shockers/logs/{shockerId}, allowing users to view historical log entries for individual shockers. The implementation includes a new store for managing log data, a logs page with a sortable data table, and a menu button on shocker control cards to navigate to the logs.
Key changes:
- New
ShockerLogStoreto fetch and cache shocker logs - New logs page component with table displaying time, type, controller, intensity, and duration
- Added dropdown menu to shocker control cards with "View Logs" option
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
src/lib/stores/ShockerLogStore.ts |
New store for fetching and caching shocker logs with Map-based storage |
src/routes/(authenticated)/shockers/logs/[shockerId=guid]/+page.svelte |
New page displaying logs in a sortable table with refresh functionality |
src/lib/components/ControlModules/impl/ShockerMenu.svelte |
New dropdown menu component for shocker actions |
src/lib/components/ControlModules/ClassicControlModule.svelte |
Integrated ShockerMenu into shocker card title |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/routes/(authenticated)/shockers/logs/[shockerId=guid]/+page.svelte
Outdated
Show resolved
Hide resolved
src/routes/(authenticated)/shockers/logs/[shockerId=guid]/+page.svelte
Outdated
Show resolved
Hide resolved
src/routes/(authenticated)/shockers/logs/[shockerId=guid]/+page.svelte
Outdated
Show resolved
Hide resolved
|
My broad questions are
|
In general I more so wanted to make a general log page, that just has all logs combined, instead of per shocker. If you wanna adpot that idea, feel free, if not then thats just gonna be at a later point :D Thank you for contributing :3 |
fix: wording Co-authored-by: LucHeart <luc@luc.cat> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
a7f178b to
b507c13
Compare
|
force pushed to sign my commit. ready for re-review @LucHeart |
|
This looks great, will approve once this gets updated to use the new endpoint 😄 |
|
I used |
@TuTiDore this seems to be something that openapi generator wants to add and remove all the time i dont know why, ive defaulted to just ignoring when openapi generator wants to remove it and just never commit it 😄 |
hhvrc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
LGTM 😄
resolves #144
/shockers/logs//shockers/logs/{id}