The Wayback Machine - https://web.archive.org/web/20220321152741/https://github.com/Automattic/wp-calypso/issues/59955
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

Welcome Tour: usePrefetchTourAssets refactor #59955

Open
escapemanuele opened this issue Jan 11, 2022 · 3 comments
Open

Welcome Tour: usePrefetchTourAssets refactor #59955

escapemanuele opened this issue Jan 11, 2022 · 3 comments

Comments

@escapemanuele
Copy link
Contributor

@escapemanuele escapemanuele commented Jan 11, 2022

Details

usePrefetchTourAssets needs to be refactored to take into consideration future tour-kit variants.
Things to work on:

  • imgSrc will be optional, because a tour could be without images. It'd currently break our hook.
  • imgSrc.desktop is assumed to be present and the default value, but it will need to be optional.

imgSrc (inside meta) type will be something like:

meta: {
	heading: string | null;
	descriptions: {
		desktop: string | null;
		mobile: string | null;
	};
	imgSrc?: {
		desktop?: {
			src: string;
			type: string;
		};
		mobile?: {
			src: string;
			type: string;
		};
	};
};

Checklist

No response

Related

No response

@StephNathai
Copy link

@StephNathai StephNathai commented Jan 14, 2022

Can I take a stab at this?

@alshakero
Copy link
Member

@alshakero alshakero commented Jan 17, 2022

Hi @StephNathai! Yes please do. If you need any help, feel free to reach out!

@StephNathai
Copy link

@StephNathai StephNathai commented Jan 20, 2022

Hi @alshakero. I have a solution for this but unfortunately due to my companies proxy settings, I am unable to complete the install to run the repo/run the tests. My code is only adding null checks to account for the optional scenarios. Will creating a PR run the tests automatically? If so, please let me know if it's ok to proceed with the PR or if I should pass on this issue, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment