Acrobat splash screen #114
Conversation
|
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
Commits |
aishwaryamathuria
requested review from
amitbikram,
Ruchika4,
sanjayms01 and
robert-bogos
and removed request for
amitbikram and
Ruchika4
| } | ||
| } | ||
|
|
||
| @media screen and (min-width: 600px) { |
There was a problem hiding this comment.
we should build our way up, the declarations in this media query should be applied by default and declarations for tablet and desktop would live in a media query
There was a problem hiding this comment.
@narcis-radu Made the suggested change
| @media screen and (max-width: 600px) { | ||
| .splash-loader .text .icon-area img { | ||
| width: 104px; | ||
| min-width: 104px; |
There was a problem hiding this comment.
I think we can remove this one and rely on the width declaration. Do you see any usage for this particular declaration?
There was a problem hiding this comment.
made the suggested change
| .then((resArr) => { | ||
| this.progressUpdater(this.splashScreenEl, 100); | ||
| const response = resArr[resArr.length - 1]; | ||
| if (!response?.url) throw new Error("Error connecting to App"); |
There was a problem hiding this comment.
should we provide more details about this error ?
There was a problem hiding this comment.
I believe wherever we are throwing Errors, we will update to dispatch error event passing in message coming from the authored error spreadsheet. This will be updated in a following PR.
There was a problem hiding this comment.
@narcis-radu Currently the errors are not handled and would be handled as a part of a different PR that Sanjay would raise.
The throw events are placeholders to some of the errors that would need an error toast
| export function updateProgressBar(layer, percentage) { | ||
| const p = Math.min(percentage, 100); | ||
| const spb = layer.querySelector('.spectrum-ProgressBar'); | ||
| spb.setAttribute('value', p); |
There was a problem hiding this comment.
| spb.setAttribute('value', p); | |
| spb?.setAttribute('value', p); |
There was a problem hiding this comment.
Made the suggested change.. This function is moved directly in action-binder to remove additional load and faster splash screen preview
|
|
||
| export default function createProgressBar() { | ||
| const layer = createTag('div', { class: 'progress-holder' }, pdom); | ||
| return layer; |
There was a problem hiding this comment.
no need for a const, you should be able to return the resulting element directly
There was a problem hiding this comment.
Made the suggested change.. This function is moved directly in action-binder to remove additional load and faster splash screen preview
|
@amitbikram I have made the discussed change to pull fragment loading inside action-binder and not via fragment.js and also combine the progress js and css with existing js and css files |
| } | ||
|
|
||
| .splash-loader.show { | ||
| display: flex !important; |
There was a problem hiding this comment.
do we really need to set !important here? .splash-loader.show has higher specificity than .splash-loader
https://jira.corp.adobe.com/browse/MWPW-156965
https://localhost/drafts/mathuria/acrobatfillsign - url to verify on local