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

Rework of search panel with gnome 43 source code #897

Merged
merged 9 commits into from Oct 5, 2022
Merged

Conversation

PapyElGringo
Copy link
Collaborator

Since in gnome 43 they changed a lot of code we used for search features I had issues with typings and compatibility so I decided to copy the new gnome 43 source code and add types for it and rework our current search code to use it so previous gnome 43 Material Shell user will get benefit from it.
So following files

src/layout/verticalPanel/search/searchProvider/AppSearchProvider.ts
src/layout/verticalPanel/search/searchProvider/RemoteSearchProvider.ts

are almost basic copy past & typed from gnome-shell source code

Copy link
Contributor

@HalfVoxel HalfVoxel left a comment

Choose a reason for hiding this comment

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

Cool! This is a diff with many copy-pastes, so it's hard to review.
Will this still work for Gnome < 43?

src/layout/verticalPanel/search/SearchResultEntry.ts Outdated Show resolved Hide resolved
for (const providerDisplay of this.providerDisplayMap.values()) {
providerDisplay.updateSearch([], []);
}
//this.remove_all_children();
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove debug code?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a reset code not debug but I can add a comment and remove the commented code

@@ -349,123 +349,6 @@ declare module 'ui' {
}
}

export namespace appDisplay {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remove these? The typings are still correct, even if we are not using them anymore.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well most of them aren't valid anymore and since the class now exist on our side I felt it was pointless

src/layout/verticalPanel/searchResultList.ts Show resolved Hide resolved
src/layout/verticalPanel/searchResultList.ts Show resolved Hide resolved
@PapyElGringo
Copy link
Collaborator Author

Cool! This is a diff with many copy-pastes, so it's hard to review. Will this still work for Gnome < 43?

Well it's should be ! If you are still on gnome 42 It's would be nice for you to try it

for (let prop in rawMeta) {
// we can use the serialized icon variant directly
if (prop !== 'icon')
unpackedMeta[prop] = metas[i][prop].deepUnpack();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
unpackedMeta[prop] = metas[i][prop].deepUnpack();
unpackedMeta[prop] = rawMeta[prop].deepUnpack();

const rawMeta = metas[i];
const unpackedMeta: UnpackedMeta = {};
for (let prop in rawMeta) {
// we can use the serialized icon variant directly
Copy link
Contributor

Choose a reason for hiding this comment

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

When do you do this?

@HalfVoxel
Copy link
Contributor

Good job!

@PapyElGringo
Copy link
Collaborator Author

Thanks !

@PapyElGringo PapyElGringo merged commit f612f56 into main Oct 5, 2022
@PapyElGringo PapyElGringo deleted the search_gnome_43 branch October 5, 2022 13:18
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.

None yet

2 participants