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

Bump core openmct dependency #184

Merged
merged 50 commits into from
Jul 31, 2024
Merged

Bump core openmct dependency #184

merged 50 commits into from
Jul 31, 2024

Conversation

davetsay
Copy link
Collaborator

Closes #137 , #132, #164
bump to 5.3.0-next

let component;
let _destroy = null;

const table = new FrameWatchTable(domainObject, this.openmct, this.options, this.type);

const view = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we call this currentView?

enable: true,
useAlternateControlBar: false,
rowName: '',
rowNamePlural: ''
};

const view = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we name this currentView?

element: document.createElement('div'),
priority: -1
};
const renderWhenVisible = func => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we use this anywhere else? should we make it a util?

@@ -14,12 +14,11 @@ export default class PacketQueryViewProvider {
}

view(domainObject, objectPath) {
let component;
let _destroy = null;

const view = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we change this to currentView

let component;
let _destroy = null;

const table = new PacketSummaryTable(domainObject, openmct);

const view = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we change this to currentView?

enable: true,
useAlternateControlBar: false,
rowName: '',
rowNamePlural: ''
};

const view = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

currentView

Copy link
Collaborator

@jvigliotta jvigliotta left a comment

Choose a reason for hiding this comment

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

Lots of great work here! Lots!

Left some comments/suggestions. Let's talk about the openmct vs this.openmct situation. There also seem to be a good amount of "Unused Variable" notifications for items returned from mount that we could get rid of.

Awesome work!

@davetsay
Copy link
Collaborator Author

Lots of great work here! Lots!

Left some comments/suggestions. Let's talk about the openmct vs this.openmct situation. There also seem to be a good amount of "Unused Variable" notifications for items returned from mount that we could get rid of.

Awesome work!

Thanks. yes we should make the openmct vs this.openmct right. We should also make the view/currentView right, but I'm reluctant to do that in this PR just in case it has some affect. mount can actually go away. That is what was done in core.

@jvigliotta
Copy link
Collaborator

New issue to address view -> currentView change: #186

@jvigliotta jvigliotta self-requested a review July 30, 2024 21:27
Copy link

Copy link
Collaborator

@jvigliotta jvigliotta left a comment

Choose a reason for hiding this comment

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

Well done! Let's get to testing.

@jvigliotta jvigliotta merged commit 266ed35 into main Jul 31, 2024
3 checks passed
@jvigliotta jvigliotta deleted the topic/update-core branch July 31, 2024 17:54
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.

Update OMM to use Open MCT v3.2.0
2 participants