Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Offline support for Network page #8332
base: master
Are you sure you want to change the base?
Offline support for Network page #8332
Changes from 27 commits
fe11d5b
6c140b4
0c1f414
e7eeecb
c5ec6f6
131cde6
2a9c4cb
4f473be
ae58738
2bfbe1c
857cfbe
7150436
da11a67
941ec88
852b144
433af38
4644b00
6fd98d0
665ea0d
b6df572
be63db4
91505b5
0715379
ade8b5f
f21a4f6
454ca92
b7b9679
ac726e8
2ea3044
e01bc3c
e7b5c90
6ab0884
4f31b92
04c8b41
ab8c519
a97135a
2a9cacf
4298078
a538a74
e05cd02
ae52105
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please address this TODO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While testing I noticed 2 issues :
Attaching SS for 1st point :
Online mode
Offline mode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another point is that in offline mode data of socket and http requests is grouped when it's populated.
I think we should show them in their original order as the requests came in? Perhaps we can pack them in a combined way instead, or other way is to reorder before rendering based on timestamps (once we fix the timings) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually past 2 weeks I've been on vacation, and then unwell so got very little time to investigate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The table should already sort the data by timestamp.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for (3) is there any simple way to add anther check for showing exit offline cta?
such as whenever it is disconnected, current check doesn't seem to work after page refresh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW I will be unavailable here for the next 4 days (27th - 1st Dec)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For point 3 (offline mode data disappearing after a page refresh): this is a known issue and should be fixed holistically for the offline framework. This is out of scope for this PR, so you don't need to worry about this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For point 2, can you just call
await _fetchFullDataBeforeExport()
at the beginning ofprepareOfflineScreenData
like we do at the beginning of theexportAsHarFile
method?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh ok, so only point 2 and pipeline checks needs to be addressed.
For this I had thought of doing the same i.e. calling await _fetchFullDataBeforeExport() at the beginning of prepareOfflineScreenData, but for that we will have to mark prepareOfflineScreenData as async, and it's parent and other implementations too, and await wherever it's called is that fine?
Btw I tried running by making these changes but getting below exception while _fetchFullDataBeforeExport() is called (I've put a try catch here) :
exception in prepareOfflineScreenData getIsolate: (-32000) Service connection disposed
Looks like the service connection is already disposed by the time prepareOfflineScreenData is called.