Add settings history.defaultPageSize#44651
Conversation
All code paths should return a value
Bundle size report 📦
Look at the Statoscope report for a full comparison between the commits ecf86bf and 6bdfb4b or learn more. Open explanation
|
|
The doubling up behaviour is weird. In your example of fetching 5 commits I would expect it to fetch 5, 10, 15, 20 etc commits each time you click "show more" so that it looks like it's loading 5 new commits each time. It looks like the increments themselves are going up which is a weird user experience. It was easy to spot near the end of your demo where you set the page size to 2. Is this the existing behaviour you mentioned? |
|
@ryanslade Right. Ideally it should be in increments of
The existing behaviour is not changing but with a smaller page size, this is more pronounced. |
| "history.defaultPageSize": { | ||
| "description": "Custom page size for the history tab. If set, the history tab will populate that number of commits at a time. If not set, it will populate 100 commits at a time.", | ||
| "type": "integer", | ||
| "default": 100, |
There was a problem hiding this comment.
If you set a default here then that will be used and the value is never undefined in JS, correct? Does it make sense to set a default here? Or should we not set one in JS if we have it here?
There was a problem hiding this comment.
That's a good point. I think to ease off the confusion of having a default value for a value that is named defaultPageSize, maybe we should remove the default from the schema config.
I wonder if we should change this? It's the sort of thing that will probably come back to us as a bug report. |
➕ on this. But I'd vote to have this in a separate PR. There's also efforts going on to improve pagination (cc @thenamankumar) so let's make sure to not step on each other's toes. |
|
@ryanslade @mrnugget I think the current behaviour was implemented as a UX improvement which might not look like one to us at the moment. But we should discuss this with the design team first before planning on removing it as the FilteredConnection component, which implements this logic, is used at multiple places. |
|
@thenamankumar thanks for the reminder. Yes, agree. I thought it was a legitimate bug, but just watched the video and agree, this seems like the intended behaviour, even though it's weird. |
Fixed #44431.
history.defaultPageSize.mp4
Note about returning 2x the responses on clicking the
Show morebutton:Test plan
Tested functionality locally.
Builds (should) pass.