From cd7c3815feebdf809f13cfb3190eae26d443b978 Mon Sep 17 00:00:00 2001 From: Wolfgang Beyer Date: Wed, 22 Sep 2021 14:41:40 +0200 Subject: [PATCH] App Id: move explainer note from tooltip into main UI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Screenshot: https://i.imgur.com/egg72jf.png Bug: 1222571 Change-Id: I41b30942273f3cad73e3bd843cba9d1e56b0b8b2 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3172980 Commit-Queue: Wolfgang Beyer Reviewed-by: Simon Zünd Reviewed-by: Phillis Tang --- config/gni/devtools_grd_files.gni | 1 - config/gni/devtools_image_files.gni | 1 - .../src/exclamation_mark_circle_icon.svg | 65 ------------------ front_end/core/i18n/locales/en-US.json | 11 +++- front_end/core/i18n/locales/en-XL.json | 11 +++- .../panels/application/AppManifestView.ts | 66 ++++++++++++++----- .../panels/application/appManifestView.css | 8 +++ test/e2e/application/manifest_test.ts | 9 ++- 8 files changed, 86 insertions(+), 86 deletions(-) delete mode 100644 front_end/Images/src/exclamation_mark_circle_icon.svg diff --git a/config/gni/devtools_grd_files.gni b/config/gni/devtools_grd_files.gni index 6a5f14f172..440396c548 100644 --- a/config/gni/devtools_grd_files.gni +++ b/config/gni/devtools_grd_files.gni @@ -52,7 +52,6 @@ grd_files_release_sources = [ "front_end/Images/elements_panel_icon.svg", "front_end/Images/errorWave.svg", "front_end/Images/error_icon.svg", - "front_end/Images/exclamation_mark_circle_icon.svg", "front_end/Images/feedback_thin_16x16_icon.svg", "front_end/Images/flex-direction-icon.svg", "front_end/Images/flex-nowrap-icon.svg", diff --git a/config/gni/devtools_image_files.gni b/config/gni/devtools_image_files.gni index fcf5b921d0..5f96eb2117 100644 --- a/config/gni/devtools_image_files.gni +++ b/config/gni/devtools_image_files.gni @@ -64,7 +64,6 @@ devtools_svg_sources = [ "elements_panel_icon.svg", "errorWave.svg", "error_icon.svg", - "exclamation_mark_circle_icon.svg", "feedback_thin_16x16_icon.svg", "flex-direction-icon.svg", "flex-nowrap-icon.svg", diff --git a/front_end/Images/src/exclamation_mark_circle_icon.svg b/front_end/Images/src/exclamation_mark_circle_icon.svg deleted file mode 100644 index 3b64cce93d..0000000000 --- a/front_end/Images/src/exclamation_mark_circle_icon.svg +++ /dev/null @@ -1,65 +0,0 @@ - - - - - - - - image/svg+xml - - - - - - - - - - - diff --git a/front_end/core/i18n/locales/en-US.json b/front_end/core/i18n/locales/en-US.json index ee91bfe444..124701113c 100644 --- a/front_end/core/i18n/locales/en-US.json +++ b/front_end/core/i18n/locales/en-US.json @@ -2103,7 +2103,7 @@ "message": "This is used by the browser to know whether the manifest should be updating an existing application, or whether it refers to a new web app that can be installed." }, "panels/application/AppManifestView.ts | appIdNote": { - "message": "Note: 'id' is not specified in the manifest, 'start_url' is used instead. To specify an App Id that matches the current identity, set the 'id' field to ''{PH1}''." + "message": "{PH1} {PH2} is not specified in the manifest, {PH3} is used instead. To specify an App Id that matches the current identity, set the {PH4} field to {PH5} {PH6}." }, "panels/application/AppManifestView.ts | appManifest": { "message": "App Manifest" @@ -2117,6 +2117,9 @@ "panels/application/AppManifestView.ts | backgroundColor": { "message": "Background color" }, + "panels/application/AppManifestView.ts | copyToClipboard": { + "message": "Copy to clipboard" + }, "panels/application/AppManifestView.ts | couldNotCheckServiceWorker": { "message": "Could not check service worker without a 'start_url' field in the manifest" }, @@ -2156,6 +2159,9 @@ "panels/application/AppManifestView.ts | installability": { "message": "Installability" }, + "panels/application/AppManifestView.ts | learnMore": { + "message": "Learn more" + }, "panels/application/AppManifestView.ts | manifestContainsDisplayoverride": { "message": "Manifest contains 'display_override' field, and the first supported display mode must be one of 'standalone', 'fullscreen', or 'minimal-ui'" }, @@ -2198,6 +2204,9 @@ "panels/application/AppManifestView.ts | noSuppliedIconIsAtLeastSpxSquare": { "message": "No supplied icon is at least {PH1} pixels square in PNG, SVG or WebP format, with the purpose attribute unset or set to \"any\"." }, + "panels/application/AppManifestView.ts | note": { + "message": "Note:" + }, "panels/application/AppManifestView.ts | orientation": { "message": "Orientation" }, diff --git a/front_end/core/i18n/locales/en-XL.json b/front_end/core/i18n/locales/en-XL.json index f1c955a49e..89282a083e 100644 --- a/front_end/core/i18n/locales/en-XL.json +++ b/front_end/core/i18n/locales/en-XL.json @@ -2103,7 +2103,7 @@ "message": "T̂h́îś îś ûśêd́ b̂ý t̂h́ê b́r̂óŵśêŕ t̂ó k̂ńôẃ ŵh́êt́ĥér̂ t́ĥé m̂án̂íf̂éŝt́ ŝh́ôúl̂d́ b̂é ûṕd̂át̂ín̂ǵ âń êx́îśt̂ín̂ǵ âṕp̂ĺîćât́îón̂, ór̂ ẃĥét̂h́êŕ ît́ r̂éf̂ér̂ś t̂ó â ńêẃ ŵéb̂ áp̂ṕ t̂h́ât́ ĉán̂ b́ê ín̂śt̂ál̂ĺêd́." }, "panels/application/AppManifestView.ts | appIdNote": { - "message": "N̂ót̂é: 'îd́' îś n̂ót̂ śp̂éĉíf̂íêd́ îń t̂h́ê ḿâńîf́êśt̂, 'śt̂ár̂t́_ûŕl̂' íŝ úŝéd̂ ín̂śt̂éâd́. T̂ó ŝṕêćîf́ŷ án̂ Áp̂ṕ Îd́ t̂h́ât́ m̂át̂ćĥéŝ t́ĥé ĉúr̂ŕêńt̂ íd̂én̂t́ît́ŷ, śêt́ t̂h́ê 'íd̂' f́îél̂d́ t̂ó ''{PH1}''." + "message": "{PH1} {PH2} îś n̂ót̂ śp̂éĉíf̂íêd́ îń t̂h́ê ḿâńîf́êśt̂, {PH3} íŝ úŝéd̂ ín̂śt̂éâd́. T̂ó ŝṕêćîf́ŷ án̂ Áp̂ṕ Îd́ t̂h́ât́ m̂át̂ćĥéŝ t́ĥé ĉúr̂ŕêńt̂ íd̂én̂t́ît́ŷ, śêt́ t̂h́ê {PH4} f́îél̂d́ t̂ó {PH5} {PH6}." }, "panels/application/AppManifestView.ts | appManifest": { "message": "Âṕp̂ Ḿâńîf́êśt̂" @@ -2117,6 +2117,9 @@ "panels/application/AppManifestView.ts | backgroundColor": { "message": "B̂áĉḱĝŕôún̂d́ ĉól̂ór̂" }, + "panels/application/AppManifestView.ts | copyToClipboard": { + "message": "Ĉóp̂ý t̂ó ĉĺîṕb̂óâŕd̂" + }, "panels/application/AppManifestView.ts | couldNotCheckServiceWorker": { "message": "Ĉóûĺd̂ ńôt́ ĉh́êćk̂ service worker ẃît́ĥóût́ â 'start_url' f́îél̂d́ îń t̂h́ê ḿâńîf́êśt̂" }, @@ -2156,6 +2159,9 @@ "panels/application/AppManifestView.ts | installability": { "message": "Îńŝt́âĺl̂áb̂íl̂ít̂ý" }, + "panels/application/AppManifestView.ts | learnMore": { + "message": "L̂éâŕn̂ ḿôŕê" + }, "panels/application/AppManifestView.ts | manifestContainsDisplayoverride": { "message": "M̂án̂íf̂éŝt́ ĉón̂t́âín̂ś 'display_override' f̂íêĺd̂, án̂d́ t̂h́ê f́îŕŝt́ ŝúp̂ṕôŕt̂éd̂ d́îśp̂ĺâý m̂ód̂é m̂úŝt́ b̂é ôńê óf̂ 'standalone', 'fullscreen', ór̂ 'minimal-ui'" }, @@ -2198,6 +2204,9 @@ "panels/application/AppManifestView.ts | noSuppliedIconIsAtLeastSpxSquare": { "message": "N̂ó ŝúp̂ṕl̂íêd́ îćôń îś ât́ l̂éâśt̂ {PH1} ṕîx́êĺŝ śq̂úâŕê ín̂ PNG, SVG ór̂ WebP f́ôŕm̂át̂, ẃît́ĥ t́ĥé p̂úr̂ṕôśê át̂t́r̂íb̂út̂é ûńŝét̂ ór̂ śêt́ t̂ó \"any\"." }, + "panels/application/AppManifestView.ts | note": { + "message": "N̂ót̂é:" + }, "panels/application/AppManifestView.ts | orientation": { "message": "Ôŕîén̂t́ât́îón̂" }, diff --git a/front_end/panels/application/AppManifestView.ts b/front_end/panels/application/AppManifestView.ts index e5fba592a3..e8709109ef 100644 --- a/front_end/panels/application/AppManifestView.ts +++ b/front_end/panels/application/AppManifestView.ts @@ -3,6 +3,7 @@ // found in the LICENSE file. import * as Common from '../../core/common/common.js'; +import * as Host from '../../core/host/host.js'; import * as i18n from '../../core/i18n/i18n.js'; import appManifestViewStyles from './appManifestView.css.js'; @@ -62,11 +63,28 @@ const UIStrings = { appIdExplainer: 'This is used by the browser to know whether the manifest should be updating an existing application, or whether it refers to a new web app that can be installed.', /** + *@description Text which is a hyperlink to more documentation + */ + learnMore: 'Learn more', + /** *@description Explanation why it is advisable to specify an 'id' field in the manifest. - *@example {https://example.com/} PH1 + *@example {Note:} PH1 + *@example {id} PH2 + *@example {start_url} PH3 + *@example {id} PH4 + *@example {/index.html} PH5 + *@example {(button for copying suggested value into clipboard)} PH6 */ appIdNote: - 'Note: \'id\' is not specified in the manifest, \'start_url\' is used instead. To specify an App Id that matches the current identity, set the \'id\' field to \'\'{PH1}\'\'.', + '{PH1} {PH2} is not specified in the manifest, {PH3} is used instead. To specify an App Id that matches the current identity, set the {PH4} field to {PH5} {PH6}.', + /** + *@description Label for reminding the user of something important. Is shown in bold and followed by the actual note to show the user. + */ + note: 'Note:', + /** + *@description Tooltip text that appears when hovering over a button which copies the previous text to the clipboard. + */ + copyToClipboard: 'Copy to clipboard', /** *@description Text for the description of something */ @@ -552,24 +570,42 @@ export class AppManifestView extends UI.Widget.VBox implements SDK.TargetManager UI.ARIAUtils.setAccessibleName(appIdField, 'App Id'); appIdField.textContent = appId; - if (!stringProperty('id')) { - const exclamationIcon = new IconButton.Icon.Icon(); - exclamationIcon.data = { - iconName: 'exclamation_mark_circle_icon', - color: 'var(--color-text-secondary)', - width: '16px', - height: '16px', - }; - exclamationIcon.classList.add('inline-icon'); - exclamationIcon.title = i18nString(UIStrings.appIdNote, {PH1: startURL}); - appIdField.appendChild(exclamationIcon); - } - const helpIcon = new IconButton.Icon.Icon(); helpIcon.data = {iconName: 'help_outline', color: 'var(--color-text-secondary)', width: '16px', height: '16px'}; helpIcon.classList.add('inline-icon'); helpIcon.title = i18nString(UIStrings.appIdExplainer); appIdField.appendChild(helpIcon); + + appIdField.appendChild(UI.XLink.XLink.create( + 'https://developer.chrome.com/blog/pwa-manifest-id/', i18nString(UIStrings.learnMore), 'learn-more')); + + if (!stringProperty('id')) { + const suggestedIdNote = appIdField.createChild('div', 'multiline-value'); + const noteSpan = document.createElement('b'); + noteSpan.textContent = i18nString(UIStrings.note); + const idSpan = document.createElement('code'); + idSpan.textContent = 'id'; + const idSpan2 = document.createElement('code'); + idSpan2.textContent = 'id'; + const startUrlSpan = document.createElement('code'); + startUrlSpan.textContent = 'start_url'; + const suggestedIdSpan = document.createElement('code'); + suggestedIdSpan.textContent = startURL; + + const copyButton = new IconButton.IconButton.IconButton(); + copyButton.title = i18nString(UIStrings.copyToClipboard); + copyButton.data = { + groups: [{iconName: 'copy_icon', iconHeight: '12px', iconWidth: '12px', text: ''}], + clickHandler: (): void => { + Host.InspectorFrontendHost.InspectorFrontendHostInstance.copyText(startURL); + }, + compact: true, + }; + + suggestedIdNote.appendChild(i18n.i18n.getFormatLocalizedString( + str_, UIStrings.appIdNote, + {PH1: noteSpan, PH2: idSpan, PH3: startUrlSpan, PH4: idSpan2, PH5: suggestedIdSpan, PH6: copyButton})); + } } this.startURLField.removeChildren(); diff --git a/front_end/panels/application/appManifestView.css b/front_end/panels/application/appManifestView.css index b19e727415..6fea8d1ff1 100644 --- a/front_end/panels/application/appManifestView.css +++ b/front_end/panels/application/appManifestView.css @@ -21,3 +21,11 @@ margin-left: 4px; vertical-align: middle; } + +.multiline-value { + white-space: normal; +} + +.learn-more { + padding-left: 4px; +} diff --git a/test/e2e/application/manifest_test.ts b/test/e2e/application/manifest_test.ts index 1f083d857f..833c8089dc 100644 --- a/test/e2e/application/manifest_test.ts +++ b/test/e2e/application/manifest_test.ts @@ -30,7 +30,7 @@ describe.skip('The Manifest Page', async () => { const fieldNames = await getTrimmedTextContent(FIELD_NAMES_SELECTOR); const fieldValues = await getTrimmedTextContent(FIELD_VALUES_SELECTOR); assert.strictEqual(fieldNames[3], 'App Id'); - assert.strictEqual(fieldValues[3], `https://localhost:${getTestServerPort()}/some_id`); + assert.strictEqual(fieldValues[3], `https://localhost:${getTestServerPort()}/some_idLearn more`); }); it('shows start id as app id', async () => { @@ -43,6 +43,11 @@ describe.skip('The Manifest Page', async () => { const fieldValues = await getTrimmedTextContent(FIELD_VALUES_SELECTOR); assert.strictEqual(fieldNames[3], 'App Id'); assert.strictEqual( - fieldValues[3], `https://localhost:${getTestServerPort()}/test/e2e/resources/application/some_start_url`); + fieldValues[3], + `https://localhost:${getTestServerPort()}/test/e2e/resources/application/some_start_url` + + 'Learn moreNote: id is not specified in the manifest, start_url is used instead. To specify an ' + + 'App Id that matches the current identity, set the id field to some_start_url .', + ); + await waitFor('icon-button[title="Copy to clipboard"]'); }); });