Private/waabusea/move media to resources #2391
Conversation
…te/waabusea/MoveMediaToResources
|
Issue #460898 is not valid. Please make sure you link an issue that exists, is open and is approved. |
| @@ -88,6 +88,11 @@ | |||
| "id": "4af62c1c-2f6d-4574-ba34-bb5c149cdf93", | |||
| "name": "Guided Experience Test", | |||
| "publisher": "Microsoft" | |||
| }, | |||
| { | |||
| "id": "d3433b68-4901-445f-9547-fdfeca57575a", | |||
There was a problem hiding this comment.
I don't think we should encourage this, using internalsvisibleto can easily get out of hand.
Create an API in the GuidedExperience Facade and use ModuleInfo to block non-Microsoft from invoking it.
There was a problem hiding this comment.
True, I was just considering if this is something we would like to make internal or not. Probably no need and I will switch it to public.
| if MediaResources.Get('COPILOTNOTAVAILABLE.PNG') then | ||
| BannerVisible := MediaResources."Media Reference".HasValue; | ||
| BannerVisible := GuidedExperience.LoadFileFromTenantMediaSet(TenantMediaSet, 'CopilotNotAvailable.png'); |
There was a problem hiding this comment.
Not sure I understand, why are we loading Media using procedures from GuidedExperience here?
There was a problem hiding this comment.
We are loading wizard images from tenant media set, there is a fine line of ownership I agree. but it is the best place I could find
| if MediaResourcesStandard.Get('ASSISTEDSETUP-NOTEXT-400PX.PNG') and | ||
| MediaResourcesDone.Get('ASSISTEDSETUPDONE-NOTEXT-400PX.PNG') and (CurrentClientType() = ClientType::Web) | ||
| then | ||
| TopBannerVisible := MediaResourcesDone."Media Reference".HasValue(); |
There was a problem hiding this comment.
Same here, are we taking dependency from Email module to Guided Experience to load Media?
I see we have a System Module called Image, might be more suitable for this purpose
| /// <summary> Loads Assisted Setup top banners from tenant media set.</summary> | ||
| /// <param name="TenantMediaSetStandard">Tenant Media set record by var for standard image.</param> | ||
| /// <param name="TenantMediaSetCompleted">Tenant Media set record by var for done image.</param> | ||
| /// <param name="TenantMediaSetInfo">Tenant Media set record by var for info image.</param> | ||
| /// <returns>True if the top banners are loaded successfuly</returns> | ||
| procedure LoadTopBanners(var TenantMediaSetStandard: Record "Tenant Media Set"; var TenantMediaSetCompleted: Record "Tenant Media Set"; var TenantMediaSetInfo: Record "Tenant Media Set"): Boolean | ||
| begin | ||
| exit(GuidedExperienceImpl.LoadTopBanners(TenantMediaSetStandard, TenantMediaSetCompleted, TenantMediaSetInfo)); | ||
| end; | ||
|
|
||
| /// <summary> Loads an image from tenant media set.</summary> | ||
| /// <param name="TenantMediaSet">Tenant Media set record by var for standard image.</param> | ||
| /// <param name="FileName">Image name.</param> | ||
| /// <returns>True if the the image is loaded successfuly</returns> | ||
| procedure LoadFileFromTenantMediaSet(var TenantMediaSet: Record "Tenant Media Set"; FileName: Text[250]): Boolean | ||
| var | ||
| begin | ||
| exit(GuidedExperienceImpl.LoadFileFromTenantMediaSet(TenantMediaSet, FileName)); | ||
| end; | ||
|
|
||
| /// <summary> Insert an image to tenant media set from system app resources.</summary> | ||
| /// <param name="TenantMediaSet">Tenant Media set record by var for standard image.</param> | ||
| /// <param name="FilePath">Path to file name.</param> | ||
| /// <param name="FileName">File name.</param> | ||
| /// <returns>True if the the image is loaded successfuly</returns> | ||
| internal procedure InsertSystemFileToTenantMediaSet(var TenantMediaSet: Record "Tenant Media Set"; FilePath: Text[100]; FileName: Text[250]) | ||
| var | ||
| begin | ||
| GuidedExperienceImpl.InsertSystemFileToTenantMediaSet(TenantMediaSet, FilePath, FileName); | ||
| end; | ||
|
|
There was a problem hiding this comment.
Not sure how those procedures fit in Guided Experience, consider Image module per previous comment
There was a problem hiding this comment.
First one, is strictly for banners in a wizard, and should definitely be there. The problem with the rest is that they are not really only for images, as resources can be anything in the future
|
Before the images are removed from the And please update the new documentation for wizard creation then, too. As soon as the examples is updated and you provide a good mechanis to get those images in custom wizards then we're able to refactor our code and update common snippets (https://github.com/waldo1001/crs-al-language-extension/blob/master/snippets/page.json) |
|
As @pri-kise said, it needs to follow the proper obsoletion path. But I'm not sure how to notify partners in pipeline about this change. Without this, you can be sure there will be many complains once this is removed from tenant media as there is certainly hundreds of apps that are using these resources as it was (still is) part of the learning path and official examples how to build this kind of pages. |
Summary
Media files will not exists in "Media resources" table anymore, instead will be added as resources to the appropriate app. For system layer, adding wizard images to guided experience. As well as copilot image when service is not available
Fixes #460898