Display custom Apps' logos #3749
Conversation
lkostrowski
changed the title
| @@ -39,6 +41,18 @@ export const AppInstallPage: React.FC<AppInstallPageProps> = ({ | |||
| }) => { | |||
| const intl = useIntl(); | |||
| const classes = useStyles(); | |||
| const { theme } = useTheme(); | |||
|
|
|||
| const getSaleorLogoUrl = () => { | |||
There was a problem hiding this comment.
We already have a themeable Saleor logo in the side navigation. Is the code duplicated, or was there another reason?
There was a problem hiding this comment.
Previously it was using a fixed image. In this diff I replaced it to one that is used in the navigation. Its not themable - these are 2 different PNGs
|
There is only one view that does not have an app img - Manage App - is it on purpose? |
|
App img during the installation process is not visible on the app list view from the start - default img is visible and then a new one is placed in its place or sometimes new img is not replacing the default one at all Nagranie.z.ekranu.2023-06-13.o.16.44.02.mov |
|
Not all apps placeholders images are visible on the list view after installation - for example slack. When entering details and moving back placeholder img is visible for a while |
It was never there, but its a good time add it 👍 |
There was a problem hiding this comment.
Removed test. It tests nothing and is very flaky
src/fragments/apps.ts
Outdated
| brand { | ||
| logo { | ||
| default | ||
| } | ||
| } |
There was a problem hiding this comment.
We should set logo thumbnail size and format arguments - it can significantly reduce loading times. Something like this should work:
default(size: 64, format:WEBP)sorry for not mentioning this earlier
There was a problem hiding this comment.
TIL :) I will update it
There was a problem hiding this comment.
@przlada updated. base64 looks broken to me, please verify
lkostrowski
dismissed stale reviews from Droniu, krzysztofwolski, and wojteknowacki
via
450d8ee
* wip * implement logo in all contexts * fix types * Improve icons rendering * Reuse App Header in Manage App page * Remove loading fallback icon for official Saleor apps. It will be fetched with manifest now * Imprve app fallback icon * wip fix tests * Remove useless test * Add sizes for logos queries * Fix thumbnail size
Displays logo from Manifest (available from 3.14) in apps list, app installation and app details
To be tested with App with logo:
https://saleor-app-products-feed-git-app-logo-saleorcommerce.vercel.app/api/manifest
or any preview URLs from this PR saleor/apps#552 (comment)
(copy preview URL + add
/api/manifestQA:
brand.logo.defaultfield in manifest will not display custom logo - will fallback to generic iconbrand.logo.defaultwill use this logo in following places: app installation page, app list - during installation, app list - after installation, app detailsOther changes
Screenshots
Pull Request Checklist
data-test-idare added for new elementsTest environment config
API_URI=https://automation-dashboard.staging.saleor.cloud/graphql/
APPS_MARKETPLACE_API_URI=https://apps.staging.saleor.io/api/v2/saleor-apps
Do you want to run more stable tests?
To run all tests, just select the stable checkbox. To speed up tests, increase the number of containers. Tests will be re-run only when the "run e2e" label is added.
CONTAINERS=1