[feedback-widget] feat: lazy load feedback widget component #13616

pieh · 2019-04-25T00:11:54Z

This is PR against #13550

After running webpagetest:

current code - https://www.webpagetest.org/result/190424_CA_151d1ef08f83ff5d1da8e8fa119dc778/
lazy loaded widget - https://www.webpagetest.org/result/190424_YE_957ff7c9ee1556fec219362c60fefa0e/

first interactive seems significantly faster on lazy loaded one and feedback component isn't critical to be immediately shown on initial mount / page transition so I think tradeoff of (slightly) delaying it in favour of faster overall interactivity seems good idea

Preview for this version available at https://5cc0c487ba5e4e79a0a62cf8--gatsby-org-pieh-previews.netlify.com/docs/

There’s more to do, but this is a start… - remove `font-family: sans-serif` along the way - remove components/feedback-widget/presets in favor of utils/presets.breakpoints

KyleAMathews · 2019-04-25T00:23:22Z

Could we lazy load it on hover or click?

KyleAMathews · 2019-04-25T00:23:37Z

onMouseOver actually?

pieh · 2019-04-25T00:29:04Z

Could we lazy load it on hover or click?
onMouseOver actually?

Sure, would require some reorganization (as we would want to show button always) and start lazy loading on hover (and focus :) ). Probably would also need some extra handling to queue click events if it wan't loaded yet to not have glitchy experience

pieh · 2019-04-25T00:29:28Z

I can address this tomorrow

- chose to reduce the border around the „emoji toggle buttons“ from 3 to 1px - add horizontal margin for screens below desktop/lg

Ref. gatsbyjs#13388

…ack-widget

…user has no intention of using it

pieh · 2019-04-25T14:55:04Z

Preview link after addressing lazy loading on hover/focus - https://5cc1c705443c86d19f93cd46--gatsby-org-pieh-previews.netlify.com/docs/

pieh · 2019-04-25T19:06:55Z

Here's preview of it - lazy loading starts when button is hovered or focused

On slow connection (as shown with emulated 2g connection above) it could be useful to indicate somehow that something is happening (if after clicking the button widget doesn't load in 500-1000ms)

KyleAMathews · 2019-04-25T19:19:04Z

Maybe use intersection observer then? A little more head start. It's 23kb so not that much code.

KyleAMathews · 2019-04-25T19:20:15Z

The animations are great btw 😍

pieh · 2019-04-25T19:29:55Z

The animations are great btw 😍

All hail @greglobinski ! :)

Maybe use intersection observer then? A little more head start. It's 23kb so not that much code.

Would you want to show button before we load full widget? Because if yes, then we will still have same problem on slower connection - user can click button before widget is ready to be used. If we don't show button, then there would be this weird (IMO) interaction on mobile, when you scroll to the bottom, suddenly button would "randomly" show up (on desktop button is fixed in viewport)

pieh · 2019-04-25T19:50:35Z

In any case - I'll try to repurpose the "working" spinning icon that is used when submitting feedback

wardpeet · 2019-04-25T20:05:17Z

Because the button has a scale, the svg is pretty blurry when transitioning, it also isn't super fluent better to set the width & height of the button to the correct value and scale down instead of scaling up. 🙈

Perhaps add a spinner to the button or set the button to disabled to show that something is happening. We could also leverage suspense for this to only show this when it's taking long.

marcysutton · 2019-04-25T20:08:00Z

In testing the preview link with the keyboard and Voiceover, I can't seem to trigger the feedback widget to open with the space or enter keys.

There is also no visible focus style–possibly because of the display: flex on the button? I'm not quite sure.

pieh · 2019-04-25T20:51:59Z

Because the button has a scale, the svg is pretty blurry when transitioning, it also isn't super fluent better to set the width & height of the button to the correct value and scale down instead of scaling up. 🙈

(I think) I didn't change anything related to styling (other than moving few things around), so this potentially would need to be mentioned in main PR - #13550

Perhaps add a spinner to the button or set the button to disabled to show that something is happening. We could also leverage suspense for this to only show this when it's taking long.

I don't want to "disable" button, as it would loose focusability (I think). Suspense I think is problematic right now for us (SSR), but I can implement this by hand.

In testing the preview link with the keyboard and Voiceover, I can't seem to trigger the feedback widget to open with the space or enter keys.

There is also no visible focus style–possibly because of the display: flex on the button? I'm not quite sure.

What browser were You using? It seems to work on Chrome for me (both navigation / toggling widget with space/enter and visible focus), but I want to take another look if I can reproduce this

marcysutton · 2019-04-25T21:15:29Z

What browser were You using?

I used Safari and Chrome. It doesn't work for me in either, even without Voiceover on.

greglobinski · 2019-04-26T11:23:02Z

It seems to work on Chrome for me (both navigation / toggling widget with space/enter and visible focus)

For me too, also on FF (Ubuntu)

I used Safari and Chrome. It doesn't work for me in either,

@marcysutton Did it work for you before?

greglobinski · 2019-04-26T12:37:36Z

the svg is pretty blurry when transitioning,

@wardpeet Are you pointing the hover rotating transition of the "close-X" icon? I will fix that, thanks.

pieh · 2019-04-26T12:46:47Z

What browser were You using? It seems to work on Chrome for me (both navigation / toggling widget with space/enter and visible focus), but I want to take another look if I can reproduce this

I was able to reproduce this in Safari. This happens because the initial button is removed once full widget loads, and focus is not being transferred to toggle button from full widget. It's pretty tough tho, because re-focusing button has visible drop-down animation (suddenly orange "outline" disappear and is animated back in).

I hoped using same ref would prevent that, but it doesn't and this is result of react diffing algorithm. TBD what's best way to solve this - either prevent focus outline animation on initial widget render or hoist the button totally, so it never change location in component tree.

…aded within 1 second of a button click

pieh · 2019-05-02T14:42:24Z

I forgot to add preview for latest version of it - https://5cc73d8862760198feb5f6c5--gatsby-org-pieh-previews.netlify.com/docs/

It should also fix focus/hover box shadow on toggle button in Safari

wardpeet

@greglobinski yes indeed :)

@pieh looks good. I would keep the focus inside the feedback widget until closed but that's another PR :)

wardpeet · 2019-05-03T10:14:31Z

www/src/components/feedback-widget/lazy-feedback-widget.js

+        }
+        return state
+      })
+    }, 1000)


following rail practices https://developers.google.com/web/fundamentals/performance/rail this should be 100ms.

I have no strong opinion on this (but I think 100ms might be to short?) - I've used timeout we use already in core

gatsby/packages/gatsby/cache-dir/navigation.js

Lines 75 to 82 in f4e8d61

// Start a timer to wait for a second before transitioning and showing a

// loader in case resources aren't around yet.

const timeoutId = setTimeout(() => {

emitter.emit(`onDelayedLoadPageResources`, { pathname })

apiRunner(`onRouteUpdateDelayed`, {

location: window.location,

})

}, 1000)

for plugins like gatsby-plugin-nprogress to show loader on page transitions

marcysutton · 2019-06-05T16:24:28Z

What should we do with this PR @pieh?

pieh · 2019-06-05T18:54:57Z

What should we do with this PR @pieh?

Oh, hmm, if this didn't prove to be problem we can just discard this PR (if we will want to get back to it - I will keep branch around, tho it's pretty outdated at this point)

Marcy Sutton and others added 10 commits April 3, 2019 20:06

initial feedback widget integration

2f1463f

Merge remote-tracking branch 'origin/master' into feedback-widget

aa4f282

update widget with greg's fabulous styles + state

175b461

add emotion plugin to jest-preprocess

0e9a1c8

improve focus management in screens

221debf

Merge remote-tracking branch 'origin/master' into feedback-widget

cb7cf17

remove widget from features page

bef73a2

Use design tokens (1/2)

d7e91a4

There’s more to do, but this is a start… - remove `font-family: sans-serif` along the way - remove components/feedback-widget/presets in favor of utils/presets.breakpoints

fix: height of widget

aa7028a

feat: lazy load feedback widget

b05299d

pieh requested a review from a team as a code owner April 25, 2019 00:11

KyleAMathews previously approved these changes Apr 25, 2019

View reviewed changes

fk and others added 6 commits April 25, 2019 13:08

Use design tokens (2/2)

b5a84b8

- chose to reduce the border around the „emoji toggle buttons“ from 3 to 1px - add horizontal margin for screens below desktop/lg

Fix max-width, overlapping navigation button for small screens

fe5b5e7

Merge branch 'master' into feedback-widget

a816c8f

Swap breakpoints -> `mediaQueries

2e26800

Ref. gatsbyjs#13388

Merge remote-tracking branch 'origin/feedback-widget' into lazy-feedb…

c168f29

…ack-widget

lazy load widget on hover/focus to not waste bandwidth/cpu cycles if …

bbd2065

…user has no intention of using it

pieh added 2 commits April 26, 2019 18:24

handle focus transfer, display loading state if lazy widget is not lo…

4a5b175

…aded within 1 second of a button click

fix: remove padding in button - fixes drop shadow in safari

d7d6b77

wardpeet reviewed May 3, 2019

View reviewed changes

greglobinski mentioned this pull request May 8, 2019

fix(www): improve feedback-widget toggle button and hover state #13923

Merged

pieh dismissed KyleAMathews’s stale review via d7d6b77 May 18, 2019 01:49

pieh changed the base branch from feedback-widget to master May 18, 2019 01:49

pieh closed this Jun 5, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feedback-widget] feat: lazy load feedback widget component #13616

[feedback-widget] feat: lazy load feedback widget component #13616

pieh commented Apr 25, 2019

KyleAMathews commented Apr 25, 2019

KyleAMathews commented Apr 25, 2019

pieh commented Apr 25, 2019 •

edited

Loading

pieh commented Apr 25, 2019

pieh commented Apr 25, 2019

pieh commented Apr 25, 2019

KyleAMathews commented Apr 25, 2019

KyleAMathews commented Apr 25, 2019

pieh commented Apr 25, 2019

pieh commented Apr 25, 2019

wardpeet commented Apr 25, 2019

marcysutton commented Apr 25, 2019

pieh commented Apr 25, 2019

marcysutton commented Apr 25, 2019 •

edited

Loading

greglobinski commented Apr 26, 2019

greglobinski commented Apr 26, 2019 •

edited

Loading

pieh commented Apr 26, 2019

pieh commented May 2, 2019

wardpeet left a comment

wardpeet May 3, 2019

pieh May 3, 2019

marcysutton commented Jun 5, 2019

pieh commented Jun 5, 2019

	// Start a timer to wait for a second before transitioning and showing a
	// loader in case resources aren't around yet.
	const timeoutId = setTimeout(() => {
	emitter.emit(`onDelayedLoadPageResources`, { pathname })
	apiRunner(`onRouteUpdateDelayed`, {
	location: window.location,
	})
	}, 1000)

[feedback-widget] feat: lazy load feedback widget component #13616

[feedback-widget] feat: lazy load feedback widget component #13616

Conversation

pieh commented Apr 25, 2019

KyleAMathews commented Apr 25, 2019

KyleAMathews commented Apr 25, 2019

pieh commented Apr 25, 2019 • edited Loading

pieh commented Apr 25, 2019

pieh commented Apr 25, 2019

pieh commented Apr 25, 2019

KyleAMathews commented Apr 25, 2019

KyleAMathews commented Apr 25, 2019

pieh commented Apr 25, 2019

pieh commented Apr 25, 2019

wardpeet commented Apr 25, 2019

marcysutton commented Apr 25, 2019

pieh commented Apr 25, 2019

marcysutton commented Apr 25, 2019 • edited Loading

greglobinski commented Apr 26, 2019

greglobinski commented Apr 26, 2019 • edited Loading

pieh commented Apr 26, 2019

pieh commented May 2, 2019

wardpeet left a comment

Choose a reason for hiding this comment

wardpeet May 3, 2019

Choose a reason for hiding this comment

pieh May 3, 2019

Choose a reason for hiding this comment

marcysutton commented Jun 5, 2019

pieh commented Jun 5, 2019

pieh commented Apr 25, 2019 •

edited

Loading

marcysutton commented Apr 25, 2019 •

edited

Loading

greglobinski commented Apr 26, 2019 •

edited

Loading