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

Full angular removal #170

Merged
merged 44 commits into from
May 3, 2024
Merged

Full angular removal #170

merged 44 commits into from
May 3, 2024

Conversation

jvigliotta
Copy link
Collaborator

@jvigliotta jvigliotta commented Apr 19, 2024

closes #108

This PR removes any remaining legacy code (openmct-legacy-support and angular mainly).

@jvigliotta jvigliotta marked this pull request as ready for review April 25, 2024 23:48
loader.js Show resolved Hide resolved
Copy link
Collaborator

@davetsay davetsay left a comment

Choose a reason for hiding this comment

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

Wow good stuff. That's a bunch of work. I made some suggestions and had some questions.

  • I think we should just be consistent and remove the this._openmct references.
  • We may want to file a follow up to improve venues, but should first find out how they really should work properly.
  • Do any unit tests need updating?
  • One thing i know was used in legacy repo, was the Data Product viewer, so let's test that.

src/exportDataAction/ExportDataAction.js Outdated Show resolved Hide resolved
src/exportDataAction/ExportDataAction.js Outdated Show resolved Hide resolved
src/exportDataAction/ExportDataTask.js Outdated Show resolved Hide resolved
src/exportDataAction/ExportDataTask.js Outdated Show resolved Hide resolved
src/AMMOSPlugins.js Outdated Show resolved Hide resolved
src/venues/Venue.js Outdated Show resolved Hide resolved
src/venues/Venue.js Outdated Show resolved Hide resolved
src/venues/Venue.js Outdated Show resolved Hide resolved
src/venues/VenueService.js Show resolved Hide resolved
};
},
watch: {
filter: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

i think we should leverage Vue to do front end filtering. I think what is happening here is we are watching for changes in filters, setting a timeout which probably has something to do with user typing speed, and then applying the filter, which is a back end filtered request.

Going further if we use the core telemetry table component, we'd have built in front end filtering.

Going even further, we probably should be using the existing historical session selector.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, you can pretty much ignore my comment above.
Venues in general don't need to be a main focus for this PR. We can get this in and improve venues down the road.

@jvigliotta
Copy link
Collaborator Author

Follow up issues:
Update Tests: #172
Optimize Venue Pllugin: #173

@jvigliotta jvigliotta requested a review from davetsay May 3, 2024 21:08
Copy link
Collaborator

@davetsay davetsay left a comment

Choose a reason for hiding this comment

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

i think the valid types thing is the only thing that needs fixing

src/exportDataAction/ExportDataAction.js Outdated Show resolved Hide resolved
'packetSummaryEventStreamUrl',
'commandEventUrl',
'commandEventStreamUrl',
'messageStreamUrl',
'frameSummaryStreamUrl',
];
Copy link
Collaborator

Choose a reason for hiding this comment

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

does some stuff need to be back ported into the constants file?

@jvigliotta jvigliotta requested a review from davetsay May 3, 2024 22:16
Copy link
Collaborator

@davetsay davetsay left a comment

Choose a reason for hiding this comment

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

Great work @jvigliotta . We finally did it!

@davetsay davetsay merged commit 65d264f into main May 3, 2024
2 checks passed
@davetsay davetsay deleted the full-angular-removal branch May 3, 2024 23:28
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.

Remove remaining legacy services
2 participants