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/beacons frontend #733

Merged
merged 7 commits into from
Aug 10, 2023
Merged

Feature/beacons frontend #733

merged 7 commits into from
Aug 10, 2023

Conversation

aaronchongth
Copy link
Member

What's new

  • Beacons micro-app
  • Added missing declarations for the beacons subscription backend
  • Updated api-client with beacon related routes
simplescreenrecorder-2023-08-08_17.01.41.mp4

Testing

Either

  • modify dashboard/src/components/robots/robots-workspace.tsx to show the beacons micro app
  • or just use a custom tab to display the microapp

Self-checks

  • I have prototyped this new feature (if necessary) on Figma
  • I'm familiar with and follow this Typescript guideline
  • I added unit-tests for new components
  • I tried testing edge cases
  • I tested the behavior of the components that interact with the backend, with an e2e test

@codecov
Copy link

codecov bot commented Aug 8, 2023

Codecov Report

Merging #733 (7cd343d) into main (0aa093d) will decrease coverage by 0.02%.
The diff coverage is 52.63%.

@@            Coverage Diff             @@
##             main     #733      +/-   ##
==========================================
- Coverage   54.65%   54.64%   -0.02%     
==========================================
  Files         261      263       +2     
  Lines        6486     6524      +38     
  Branches      838      862      +24     
==========================================
+ Hits         3545     3565      +20     
- Misses       2803     2819      +16     
- Partials      138      140       +2     
Flag Coverage Δ
dashboard 18.44% <0.00%> (-0.15%) ⬇️
react-components 52.12% <90.90%> (+0.51%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
packages/dashboard/src/components/app-registry.ts 0.00% <ø> (ø)
packages/dashboard/src/components/beacons-app.tsx 0.00% <0.00%> (ø)
...react-components/lib/doors/door-table-datagrid.tsx 74.28% <ø> (ø)
...react-components/lib/lifts/lift-table-datagrid.tsx 56.41% <ø> (ø)
...t-components/lib/beacons/beacon-table-datagrid.tsx 90.90% <90.90%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@Angatupyry Angatupyry left a comment

Choose a reason for hiding this comment

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

Thanks for doing this, Aaron!
This code follows the way that we have designed Doors and Lifts tables so I have not any big comments.
I left only one nit comment.
All clean and looks really nice!

import { ApiServerModelsTortoiseModelsBeaconsBeaconStateLeaf as BeaconState } from 'api-client';
import { DataGrid, GridColDef, GridValueGetterParams, GridCellParams } from '@mui/x-data-grid';
import { Box, SxProps, Typography, useTheme } from '@mui/material';
import * as React from 'react';
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Is there any reason to do this instead of how we do it in the rest of the files (import React from 'react';)?
In fact, we could start by doing import {useState, useEffect, ...} from React to be more specific.

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like I copied over from doors and lifts too much, thanks for catching that! 7cd343d fixes it as well as for doors and lifts

@Angatupyry
Copy link
Collaborator

@aaronchongth Oh, btw, I don't see the Beacons table in the System Overview tab, is there a reason for that?
Screenshot from 2023-08-08 14-52-14

@aaronchongth aaronchongth merged commit 5fcccd3 into main Aug 10, 2023
5 checks passed
@aaronchongth aaronchongth deleted the feature/beacons-frontend branch August 10, 2023 02:46
@aaronchongth
Copy link
Member Author

@aaronchongth Oh, btw, I don't see the Beacons table in the System Overview tab, is there a reason for that? Screenshot from 2023-08-08 14-52-14

@Angatupyry sorry I forgot to answer this, beacons will be rather specific hardware that is not used in any of our demos or use cases for now, so we'll only use them in projects or when we do have beacons in our simulation demos. For projects that use them, I expect the layout will be different anyway

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.

2 participants