Skip to content

Commit

Permalink
Transparent new default for Windows Icon Backgrounds. (#4571)
Browse files Browse the repository at this point in the history 
fixes #4401
<!-- Link to relevant issue (for ex: "fixes #1234") which will
automatically close the issue once the PR is merged -->

## PR Type
<!-- Please uncomment one ore more that apply to this PR -->

<!-- - Bugfix -->
Feature
<!-- - Code style update (formatting) -->
<!-- - Refactoring (no functional changes, no api changes) -->
<!-- - Build or CI related changes -->
<!-- - Documentation content changes -->
<!-- - Sample app changes -->
<!-- - Other... Please describe: -->


## Describe the current behavior?
<!-- Please describe the current behavior that is being modified or link
to a relevant issue. -->
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
#4550
  • Loading branch information
@Jaylyn-Barbee
Jaylyn-Barbee authored Dec 7, 2023
1 parent 31e78fb commit ccf6cdc
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 194 deletions.
2 changes: 1 addition & 1 deletion apps/pwabuilder/src/script/components/test-publish-pane.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ export class TestPublishPane extends LitElement {
<div id="pp-frame-content">
<div id="pp-frame-header">
<h1>Download Test Package</h1>
<p>If you want to see what your app would look like in its current state, use the test package buttons below!</p>
<p>If you want to see what your app would look like in its current state, use the test package button below!</p>
</div>
<div id="store-cards">
${this.renderContentCards()}
Expand Down
22 changes: 16 additions & 6 deletions apps/pwabuilder/src/script/components/windows-form.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 [
Expand Down Expand Up @@ -202,15 +203,20 @@ export class WindowsForm extends AppPackageFormBase {
if (manifestContext.isGenerated) {
manifestContext = await fetchOrCreateManifest();
}

this.packageOptions = createWindowsPackageOptionsFromManifest(
manifestContext!.manifest
);

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') {
Expand Down Expand Up @@ -327,7 +333,7 @@ export class WindowsForm extends AppPackageFormBase {
<p class="sub-multi">Select your Windows icons background color</p>
<sl-radio-group
id="icon-bg-radio-group"
.value=${this.packageOptions!.images!.backgroundColor === 'transparent' ? 'transparent' : 'custom'}
.value=${'transparent'}
@sl-change=${() => this.toggleIconBgRadios()}
>
<sl-radio class="color-radio" size="small" value="transparent">Transparent</sl-radio>
Expand All @@ -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() {
Expand Down Expand Up @@ -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,
})}
Expand Down
2 changes: 1 addition & 1 deletion apps/pwabuilder/src/script/services/publish/windows-publish.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
186 changes: 0 additions & 186 deletions components/manifest-editor/src/components/manifest-screenshots-form.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -299,139 +297,6 @@ export class ManifestScreenshotsForm extends LitElement {
}
}


renderScreenshotInputUrlList() {
const renderFn = (_url: string | undefined, index: number) => {

return html`<sl-input
placeholder="https://www.example.com/screenshot"
value="${this.baseURL || ''}"
@input=${this.handleScreenshotButtonEnabled}
@sl-change=${this.handleScreenshotUrlChange}
data-index=${index}
></sl-input>`
};

return this.screenshotUrlList.map(renderFn);
}

handleScreenshotUrlChange(event: CustomEvent) {
const input = <HTMLInputElement>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<string | undefined>) {
const results: Array<boolean> = [];
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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -558,19 +385,6 @@ export class ManifestScreenshotsForm extends LitElement {
<div class="sc-gallery">
${this.srcList.length > 0 ? this.srcList.map((img: any) => html`<img class="screenshot" src=${img} alt="your app screenshot" decoding="async" loading="lazy"/>`) : html`<div class="center_text"><sl-icon name="card-image"></sl-icon> There are no screenshots in your manifest</div>`}
</div>
<h3>Generate Screenshots</h3>
<p>Specify the URLs to generate desktop and mobile screenshots from. You may add up to 8 screenshots or Store Listings.</p>
${this.renderScreenshotInputUrlList()}
<sl-button id="add-sc" @click=${this.addNewScreenshot} ?disabled=${this.addScreenshotUrlDisabled}>+ Add URL</sl-button>
<div class="screenshots-actions">
<sl-button
type="submit"
?loading=${this.awaitRequest}
?disabled=${this.generateScreenshotButtonDisabled}
@click=${this.generateScreenshots}
>Generate Screenshots</sl-button>
</div>
<div class="sc-gallery">
${this.newSrcList.map((img: any) => html`<img class="screenshot" alt="your generated screenshot" src=${img} />`)}
</div>
Expand Down

0 comments on commit ccf6cdc

Please sign in to comment.