search

kevinmarrec / nuxt-pwa-module

⚠️ DEPRECATED ⚠️ Zero config PWA solution for Nuxt 3
MIT License
335 stars 32 forks source link

ios splash screen #4

Closed xlanex6 closed 2 years ago

xlanex6 commented 2 years ago

@kevinmarrec

Feedback could be great, as I told you , I'am rookie on this, but trust my motivation

xlanex6 commented 2 years ago

@kevinmarrec

I made a choice to set devices array as default module's options. Kept all devices by default, is it good ?

I add also some types definition due about splash key introduction in Icon

Question :

Is it make sens to keep splash-screen generation triggers by manifest.mobileAppIOS if we provide splash object in icon ? should we splash: { devices , .....} or splash : false to prevent generation ?

kevinmarrec commented 2 years ago

@xlanex6 I think we could use icon.iosSizes to keep aligned with Nuxt 2 PWA for now.

But as Nuxt 3 modules uses defu which merges configuration arrays, we need to do like https://github.com/kevinmarrec/nuxt-pwa-module/blob/main/src/icon.ts#L32 to only set icon.iosSizes to default devices if the array is empty (so that people can still override it)

Then, in both icon & meta sub modules, we should check pwa.meta.mobileAppIOS to decide if the icon generation & meta inject are needed.

So if i'm right, these wouldn't be generated by default, but everything could be opt in with mobileAppIOS: true

xlanex6 commented 2 years ago

@kevinmarrec

Based on your previous comment I implement devices array as empty by default , and inject if user not overwrite it. ( same pattern )

splash screen generation still trigged by mobileAppIOS: true ( previous commit )

kevinmarrec commented 2 years ago

@xlanex6 Just letting you know that I'll have a look on last updates only Sunday, as I'm off computer until this day :)

kevinmarrec commented 2 years ago

@xlanex6 Do you mind if I merge the current state in a splash branch here ?

I'd like to have it released by tomorrow evening, but I still have changes to request (most likely minors).

When merged in main, we'll be both co-authors of the commit so we track your contribution, what do you think ?

kevinmarrec commented 2 years ago

We could also just wait next week, as if there's any issue with the release I won't really be able to work on it this week (I'm off computer most days of this week)

xlanex6 commented 2 years ago

@kevinmarrec

Both options are good for me until my tiny contribution still there.