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

Extension search fixes #9231

Merged
merged 8 commits into from
Nov 18, 2022
Merged

Extension search fixes #9231

merged 8 commits into from
Nov 18, 2022

Conversation

jwunderl
Copy link
Member

left a few todos spread around as I didn't want to go to poke around too much besides fixing the known regressions as this will probably be ported to arcade, microbit, maybe minecraft (I'm guessing content authors could be using the share script extensions to test at least?) ~

Here's a build to double check: https://arcade.makecode.com/app/5dc92e4c8357413a5065b97411a4157ffd9c89ec-0aa863d716

image

@abchatra
Copy link
Collaborator

Question, is the logic ported from old extension code?

@jwunderl
Copy link
Member Author

jwunderl commented Nov 15, 2022

Maybe closer to call it inspired by, closest port is here with the logic for setting the dependency here https://github.com/microsoft/pxt/blob/extensionSearchFixes/webapp/src/scriptsearch.tsx#L253 which is the main portion on using setDepAsync

I didn't look at how the cards were displayed in scriptsearch.tsx as the ui implementation is just different between the two, but it was pretty straightforward to extract script card info -> card contents -- I looked at arcade.makecode.com/v1.5 and it doesn't show the thumbnail as the image or the "we're not endorsing this you know" warning, but those feel like pretty good additions to me

image

(it's also using the same cloud-search:${shareScriptId} from here https://github.com/microsoft/pxt/blob/extensionSearchFixes/webapp/src/scriptsearch.tsx#L126)

@jwunderl
Copy link
Member Author

I swapped it over to make it so loading share scripts uses the same addDepIfNoConflict path as adding extensions, new build here: https://arcade.makecode.com/app/9fd9d109f9c8727d9b30d6f8f04be9dc6e5b89c3-1eac5059bc

@@ -377,6 +428,21 @@ export const ExtensionsBrowser = (props: ExtensionsProps) => {
}
}

function extensionMetaCard(scr: ExtensionMeta & EmptyCard, baseClass: string, ind: number) {
Copy link
Member

Choose a reason for hiding this comment

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

nit: make this a functional reach component

@abchatra
Copy link
Collaborator

Once merged please port to microbit and arcade stable channels.

@jwunderl jwunderl merged commit a8a430b into master Nov 18, 2022
@jwunderl jwunderl deleted the extensionSearchFixes branch November 18, 2022 17:27
jwunderl added a commit that referenced this pull request Nov 18, 2022
* fix wrong gh extension being downloadexd

* fix shareurl loading as extension

* small cleanup thumbless share scripts

* remove more specific types that were commented out

* remove comment on delay

* use addDepIfNoConflict to avoid duping loading logic

* make ExtensionMetaCard a fc

* actually just don't need className as it was just for key
jwunderl added a commit that referenced this pull request Nov 18, 2022
* fix wrong gh extension being downloadexd

* fix shareurl loading as extension

* small cleanup thumbless share scripts

* remove more specific types that were commented out

* remove comment on delay

* use addDepIfNoConflict to avoid duping loading logic

* make ExtensionMetaCard a fc

* actually just don't need className as it was just for key
abchatra pushed a commit that referenced this pull request Nov 18, 2022
* fix wrong gh extension being downloadexd

* fix shareurl loading as extension

* small cleanup thumbless share scripts

* remove more specific types that were commented out

* remove comment on delay

* use addDepIfNoConflict to avoid duping loading logic

* make ExtensionMetaCard a fc

* actually just don't need className as it was just for key
jwunderl added a commit that referenced this pull request Nov 22, 2022
* fix wrong gh extension being downloadexd

* fix shareurl loading as extension

* small cleanup thumbless share scripts

* remove more specific types that were commented out

* remove comment on delay

* use addDepIfNoConflict to avoid duping loading logic

* make ExtensionMetaCard a fc

* actually just don't need className as it was just for key
jwunderl added a commit that referenced this pull request Nov 23, 2022
* Extension search fixes (#9231)

* fix wrong gh extension being downloadexd

* fix shareurl loading as extension

* small cleanup thumbless share scripts

* remove more specific types that were commented out

* remove comment on delay

* use addDepIfNoConflict to avoid duping loading logic

* make ExtensionMetaCard a fc

* actually just don't need className as it was just for key

* fix nested repos direct add (#9249)
jwunderl added a commit that referenced this pull request Nov 23, 2022
* Extension search fixes (#9231)

* fix wrong gh extension being downloadexd

* fix shareurl loading as extension

* small cleanup thumbless share scripts

* remove more specific types that were commented out

* remove comment on delay

* use addDepIfNoConflict to avoid duping loading logic

* make ExtensionMetaCard a fc

* actually just don't need className as it was just for key

* when no preferred extensions, don't show all approved exts (#9247)

* when no preferred extensions, don't show all approved exts

* use preferredexts instead of extToShow to fix clicking back

* Fix minecraft hoc layout bug (#9243)

* fix minecraft hoc height issue

* simplify offset height calc for old tutorials

* move conditional logic into conditional

* also update editoroffset post setTutorialInstructionExpanded

* fix nested repos direct add (#9249)
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.

Not able to search for extensions using Shared Project URL wrong extension being imported
3 participants