From ccf6cdc41f6726f7ac4706676b501e82840d6f8c Mon Sep 17 00:00:00 2001 From: Jaylyn Barbee <51131738+Jaylyn-Barbee@users.noreply.github.com> Date: Thu, 7 Dec 2023 09:55:17 -0500 Subject: [PATCH] Transparent new default for Windows Icon Backgrounds. (#4571) fixes https://github.com/pwa-builder/PWABuilder/issues/4401 ## PR Type Feature ## Describe the current behavior? Manifest background color is default for icon background ## Describe the new behavior? Transparent is default for icon background ## PR Checklist - [x] Test: run `npm run test` and ensure that all tests pass - [x] Target main branch (or an appropriate release branch if appropriate for a bug fix) - [x] Ensure that your contribution follows [standard accessibility guidelines](https://docs.microsoft.com/en-us/microsoft-edge/accessibility/design). Use tools like https://webhint.io/ to validate your changes. ## Additional Information This PR also disables the ability to generate screenshots as we feel generating base64 strings into peoples manifest is creating an inefficient system and people don't understand why its inefficient and trusting that PWABuilder is doing the right thing when it isn't. Part of the temporary solution described here https://github.com/pwa-builder/PWABuilder/issues/4550 --- .../script/components/test-publish-pane.ts | 2 +- .../src/script/components/windows-form.ts | 22 ++- .../services/publish/windows-publish.ts | 2 +- .../components/manifest-screenshots-form.ts | 186 ------------------ 4 files changed, 18 insertions(+), 194 deletions(-) diff --git a/apps/pwabuilder/src/script/components/test-publish-pane.ts b/apps/pwabuilder/src/script/components/test-publish-pane.ts index 7760f3333..900b03daf 100644 --- a/apps/pwabuilder/src/script/components/test-publish-pane.ts +++ b/apps/pwabuilder/src/script/components/test-publish-pane.ts @@ -537,7 +537,7 @@ export class TestPublishPane extends LitElement {

Download Test Package

-

If you want to see what your app would look like in its current state, use the test package buttons below!

+

If you want to see what your app would look like in its current state, use the test package button below!

${this.renderContentCards()} diff --git a/apps/pwabuilder/src/script/components/windows-form.ts b/apps/pwabuilder/src/script/components/windows-form.ts index 88eeb18e4..4b33e4c2a 100644 --- a/apps/pwabuilder/src/script/components/windows-form.ts +++ b/apps/pwabuilder/src/script/components/windows-form.ts @@ -25,6 +25,7 @@ export class WindowsForm extends AppPackageFormBase { @state() packageOptions: WindowsPackageOptions = emptyWindowsPackageOptions(); @state() activeLanguages: string[] = []; @state() activeLanguageCodes: string[] = []; + @state() userBackgroundColor: string = ""; static get styles() { return [ @@ -202,7 +203,7 @@ export class WindowsForm extends AppPackageFormBase { if (manifestContext.isGenerated) { manifestContext = await fetchOrCreateManifest(); } - + this.packageOptions = createWindowsPackageOptionsFromManifest( manifestContext!.manifest ); @@ -210,7 +211,12 @@ export class WindowsForm extends AppPackageFormBase { this.packageOptions.targetDeviceFamilies = ['Desktop', 'Holographic']; this.customSelected = this.packageOptions.images?.backgroundColor != 'transparent'; - this.initialBgColor = this.currentSelectedColor = (this.packageOptions.images?.backgroundColor as string); + this.currentSelectedColor = this.packageOptions.images?.backgroundColor!; + if(manifestContext?.manifest.background_color){ + this.initialBgColor = manifestContext!.manifest.background_color; + } else { + this.initialBgColor = "#000000;" + } } toggleSettings(settingsToggleValue: 'basic' | 'advanced') { @@ -327,7 +333,7 @@ export class WindowsForm extends AppPackageFormBase {

Select your Windows icons background color

this.toggleIconBgRadios()} > Transparent @@ -344,13 +350,17 @@ export class WindowsForm extends AppPackageFormBase { toggleIconBgRadios(){ let input = (this.shadowRoot?.getElementById("icon-bg-radio-group") as any); let selected = input.value; - this.customSelected = selected !== 'transparent'; - if(!this.customSelected){ + + // update values + if(this.customSelected){ this.packageOptions.images!.backgroundColor = 'transparent'; } else { this.packageOptions.images!.backgroundColor = this.initialBgColor; this.currentSelectedColor = this.initialBgColor; } + + // switch flag which will trigger update + this.customSelected = selected !== 'transparent'; } render() { @@ -524,7 +534,7 @@ export class WindowsForm extends AppPackageFormBase { 'https://learn.microsoft.com/en-us/windows/apps/design/style/iconography/app-icon-design#color-contrast', inputId: 'icon-bg-color-input', type: 'color', - value: this.packageOptions.images!.backgroundColor || 'transparent', + value: this.packageOptions.images?.backgroundColor!, placeholder: 'transparent', inputHandler: (val: string) => this.packageOptions.images!.backgroundColor = val, })} diff --git a/apps/pwabuilder/src/script/services/publish/windows-publish.ts b/apps/pwabuilder/src/script/services/publish/windows-publish.ts index 370d1b610..d031093b9 100644 --- a/apps/pwabuilder/src/script/services/publish/windows-publish.ts +++ b/apps/pwabuilder/src/script/services/publish/windows-publish.ts @@ -140,7 +140,7 @@ export function createWindowsPackageOptionsFromManifest( manifest: manifest, images: { baseImage: icon?.src || '', - backgroundColor: manifest.background_color || 'transparent', + backgroundColor: 'transparent', padding: 0.0, }, resourceLanguage: languages, diff --git a/components/manifest-editor/src/components/manifest-screenshots-form.ts b/components/manifest-editor/src/components/manifest-screenshots-form.ts index d79ca61d0..9964a2de5 100644 --- a/components/manifest-editor/src/components/manifest-screenshots-form.ts +++ b/components/manifest-editor/src/components/manifest-screenshots-form.ts @@ -7,7 +7,6 @@ import { Manifest, Icon, } from '../utils/interfaces'; -import { generateScreenshots } from '../utils/screenshots'; import { resolveUrl } from '../utils/urls'; import {classMap} from 'lit/directives/class-map.js'; import "./manifest-field-tooltip"; @@ -233,7 +232,6 @@ export class ManifestScreenshotsForm extends LitElement { manifestInitialized = false; this.requestValidateAllFields(); await this.screenshotSrcListParse(); - await this.newScreenshotSrcListParse(); if(this.manifest.screenshots && this.initialScreenshotLength == -1){ this.initialScreenshotLength = this.manifest.screenshots.length; } else { @@ -299,139 +297,6 @@ export class ManifestScreenshotsForm extends LitElement { } } - - renderScreenshotInputUrlList() { - const renderFn = (_url: string | undefined, index: number) => { - - return html`` - }; - - return this.screenshotUrlList.map(renderFn); - } - - handleScreenshotUrlChange(event: CustomEvent) { - const input = event.target; - const index = Number(input.dataset['index']); - - this.screenshotUrlList[index] = input.value; - this.screenshotListValid = this.validateScreenshotUrlsList( - this.screenshotUrlList - ); - this.addScreenshotUrlDisabled = !this.disableAddUrlButton(); - this.generateScreenshotButtonDisabled = !this.hasScreenshotsToGenerate(); - } - - addNewScreenshot() { - this.screenshotUrlList = [...(this.screenshotUrlList || []), undefined]; - this.addScreenshotUrlDisabled = !this.disableAddUrlButton(); - this.generateScreenshotButtonDisabled = !this.hasScreenshotsToGenerate(); - } - - disableAddUrlButton() { - return ( - this.screenshotUrlList?.length < 8 && this.hasScreenshotsToGenerate() - ); - } - - handleScreenshotButtonEnabled() { - if (this.generateScreenshotButtonDisabled === true) { - this.generateScreenshotButtonDisabled = false; - } - } - - hasScreenshotsToGenerate() { - return ( - this.screenshotUrlList.length && - !this.screenshotListValid.includes(false) && - !this.screenshotUrlList.includes(undefined) - ); - } - - validateScreenshotUrlsList(urls: Array) { - const results: Array = []; - const websiteRegex = /((([A-Za-z]{3,9}:(?:\/\/)?)(?:[-;:&=\+\$,\w]+@)?[A-Za-z0-9.-]+|(?:www.|[-;:&=\+\$,\w]+@)?[A-Za-z0-9.-]+)((?:\/[\+~%\/.\w\-_]*)?\??(?:[-\+=&;%@.\w_]*)#?(?:[\w]*))?)/; - - const length = urls.length; - for (let i = 0; i < length; i++) { - const urlToHandle = urls[i]; - results[i] = urlToHandle ? websiteRegex.test(urlToHandle) : false; - } - - return results; - } - - async generateScreenshots() { - - let generateScreenshotsAttempted = new CustomEvent('generateScreenshotsAttempted', { - bubbles: true, - composed: true - }); - this.dispatchEvent(generateScreenshotsAttempted); - - - if(this.validationPromise){ - await this.validationPromise; - } - - try { - this.awaitRequest = true; - - if (this.screenshotUrlList && this.screenshotUrlList.length) { - // to-do: take another type look at this - // @ts-ignore - const screenshots = await generateScreenshots(this.screenshotUrlList); - if (screenshots) { - this.screenshotsList = screenshots; - let manifestUpdated = new CustomEvent('manifestUpdated', { - detail: { - field: "screenshots", - change: screenshots - }, - bubbles: true, - composed: true - }); - this.dispatchEvent(manifestUpdated); - - let screenshotsUpdated = new CustomEvent('screenshotsUpdated', { - detail: { - screenshots: screenshots - }, - bubbles: true, - composed: true - }); - this.dispatchEvent(screenshotsUpdated); - - let title = this.shadowRoot!.querySelector('h3'); - if(title!.classList.contains("error")){ - title!.classList.toggle("error"); - this.errorCount--; - let errorMessage = this.shadowRoot!.querySelector(".error-message"); - errorMessage?.parentNode?.removeChild(errorMessage); - } - // In the future if we wanna show some message it can be tied to this bool - this.showSuccessMessage = true; - } - } - } catch (e) { - console.error(e); - // In the future if we wanna show some message it can be tied to this bool - this.showErrorMessage = true; - } - - this.awaitRequest = false; - if(this.errorCount == 0){ - this.dispatchEvent(errorInTab(false, "screenshots")); - } else { - this.dispatchEvent(errorInTab(true, "screenshots")); - } - } - async screenshotSrcListParse() { if (!this.manifest || !this.manifest.screenshots) { @@ -462,44 +327,6 @@ export class ManifestScreenshotsForm extends LitElement { this.srcList = srcList; } - async newScreenshotSrcListParse() { - if (!this.manifest) { - return; - } - - let srcList: string[] = []; - - if(this.manifest.screenshots && this.manifest.screenshots.length > this.initialScreenshotLength){ - - let initialCounter = this.initialScreenshotLength; - - //this.manifest!.screenshots?.forEach((sc: any) => { - for(let i = 0; i < this.manifest!.screenshots!.length; i++){ - let sc = this.manifest!.screenshots![i]; - if(initialCounter != 0){ - initialCounter--; - } else { - let scURL: string = this.handleImageUrl(sc) || ''; - - await this.testImage(scURL).then( - function fulfilled(_img) { - }, - - function rejected() { - scURL = `https://pwabuilder-safe-url.azurewebsites.net/api/getSafeUrl?url=${scURL}`; - } - ); - - if(scURL){ - srcList.push(scURL as string); - } - } - } - } - - this.newSrcList = srcList; - } - testImage(url: string) { // Define the promise @@ -558,19 +385,6 @@ export class ManifestScreenshotsForm extends LitElement {
${this.srcList.length > 0 ? this.srcList.map((img: any) => html`your app screenshot`) : html`
There are no screenshots in your manifest
`}
-

Generate Screenshots

-

Specify the URLs to generate desktop and mobile screenshots from. You may add up to 8 screenshots or Store Listings.

- ${this.renderScreenshotInputUrlList()} - + Add URL - -
- Generate Screenshots -
${this.newSrcList.map((img: any) => html`your generated screenshot`)}