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

Add inspections page #982

Closed
wants to merge 1 commit into from
Closed

Conversation

andchiind
Copy link
Contributor

@andchiind andchiind commented Aug 18, 2023

Closes #839 #944

@andchiind andchiind added feature New feature or request backend Backend related functionality frontend Frontend related functionality labels Aug 18, 2023
@github-actions
Copy link

🔔 Changes in database folder detected 🔔
Do these changes require adding new migrations? 🤔 In that case follow these steps.
If you are uncertain, ask a database admin on the team 😄

@andchiind andchiind force-pushed the inspections branch 2 times, most recently from 7ce93a8 to af18384 Compare August 18, 2023 09:24
@andchiind andchiind marked this pull request as ready for review August 18, 2023 12:58
@andchiind andchiind marked this pull request as draft August 23, 2023 11:32
@oysand
Copy link
Contributor

oysand commented Aug 24, 2023

I see this is linked to #839 which is in review, though this is still as draft. Is it ready for review?

@andchiind
Copy link
Contributor Author

I see this is linked to #839 which is in review, though this is still as draft. Is it ready for review?

I think at this point it is ready for review. It is not identical to the figma, but doing so will require more extensive changes, including database changes.

@andchiind andchiind marked this pull request as ready for review August 24, 2023 08:50
@andchiind andchiind force-pushed the inspections branch 6 times, most recently from c940331 to e426121 Compare August 25, 2023 11:54
optionLabel={(r) => r.name + ' (' + r.model.type + ')'}
options={robotOptions.filter(
(r) =>
r.currentInstallation.toLocaleLowerCase() === installationCode.toLocaleLowerCase()
Copy link
Contributor

@mrica-equinor mrica-equinor Sep 5, 2023

Choose a reason for hiding this comment

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

Suggest filtering the robots with the installation code on the use effect instead of here so that maybe the autocomplete options load a bit faster

Copy link
Contributor

Choose a reason for hiding this comment

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

I've tried and didn't help much but still think it looks better here than in the autocomplete

useEffect(() => {
const id = setInterval(() => {
BackendAPICaller.getEnabledRobots().then((robots) => robots.filter(robots => robots.currentInstallation.toLowerCase() === installationCode.toLowerCase()))
.then((robots) => {
setRobotOptions(robots)
})
}, props.refreshInterval)
return () => clearInterval(id)
}, [props.refreshInterval])

Comment on lines 91 to 116
<StyledMissionDialog>
<StyledDialog open={true}>
<StyledAutoComplete>
<StyledMissionSection>
<Button
onClick={() => {
props.closeDialog()
}}
variant="outlined"
color="primary"
>
{' '}
{TranslateText('Cancel')}{' '}
</Button>
</StyledMissionSection>
</StyledAutoComplete>
</StyledDialog>
</StyledMissionDialog>

<StyledMissionDialog>
<Dialog open={true}>
<StyledAutoComplete>
<Typography variant="h5">{TranslateText('This installation has no missions')}</Typography>
</StyledAutoComplete>
</Dialog>
</StyledMissionDialog>
Copy link
Contributor

Choose a reason for hiding this comment

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

All this can be deleted

Comment on lines 48 to 50
BackendAPICaller.getEnabledRobots().then((robots) => {
setRobotOptions(robots)
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of making the API call here, use the props that are used for echoMissions on MissionOverview/ScheduleMissionDialog so that the autocomplete is faster

Copy link
Contributor

Choose a reason for hiding this comment

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

If you change the refresh interval on this section to refreshInterval instead of slowRefreshInterval, this is no longer necessary

@mrica-equinor
Copy link
Contributor

mrica-equinor commented Sep 5, 2023

There's a bit of delay when scheduling the missions on the inspection table that also causes the scrollbar to disappear (loading issues)

Edit: If you change the refresh interval on this section to refreshInterval instead of slowRefreshInterval, this is no longer necessary

@aeshub
Copy link
Contributor

aeshub commented Sep 8, 2023

image
The schedule all button should not be present when there is a deck without missions.

@mrica-equinor
Copy link
Contributor

Discussion continues on #1004

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Backend related functionality feature New feature or request frontend Frontend related functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Visualise the inspection state for each asset deck
5 participants