Download bundle UX improvement #2453

vibros68 · 2021-06-22T10:00:38Z

resolved #2438
resolved #2461

Description

adjust download bundle: show loading wheel while the data is being fetched; show progress for paginated downloads only when there are more than 1 page

…tched; show progress for paginated downloads only when there are more than 1 page

lukebp · 2021-06-22T15:45:10Z

This doesn't appear to be updating the % complete correctly.

tmp-2021-06-22_10.40.57.mp4

You can use the pictl seedproposals [adminEmail] [adminPassword] command to seed your backend with proposals that contain 150 comments each in order to test out the paginated timestamps.

vibros68 · 2021-06-22T19:04:34Z

I fixed it. Thank you for guidance

lukebp

I'm seeing two issues.

A ticketvote/v1/details request is being made when you load the proposal details page. This is a expensive route and should only be retrieved when the votes bundle is being downloaded.
Downloading the votes bundle does not download the actual cast votes.

lukebp · 2021-06-22T19:17:08Z

src/containers/Comments/Download/hooks.js

@@ -59,7 +60,7 @@ export function useDownloadCommentsTimestamps(recordToken) {
  const getProgressPercentage = useCallback(
    (timestamps) =>
      timestamps
-        ? (Object.keys(timestamps.length * 100) / commentsLength).toFixed(2)
+        ? ((Object.keys(timestamps).length * 100) / commentsLength).toFixed(2)


I don't think we need to show decimal places on these % complete indicators. Instead of 66.67%, lets just show 67%.

victorgcramos

Hey @vibros68, thanks for the PR. Left some inline comments in addition to luke's review.

victorgcramos · 2021-06-28T20:01:04Z

src/containers/Comments/Download/DownloadCommentsTimestamps.jsx

    useDownloadCommentsTimestamps(recordToken);

  return loading ? (
    <div>
-      <span style={{ marginRight: 10 }}>{progress}%</span>
+      {multiPage ? <span style={{ marginRight: 10 }}>{progress}%</span> : <></>}


Suggested change

{multiPage ? {progress}% : <></>}

{multiPage && {progress}%}

victorgcramos · 2021-06-28T20:39:38Z

src/containers/Proposal/Download/DownloadVotesTimestamps.jsx


  return loading ? (
    <div>
-      <span style={{ marginRight: 10 }}>{progress}%</span>
+      {multiPage ? <span style={{ marginRight: 10 }}>{progress}%</span> : <></>}


Suggested change

{multiPage ? {progress}% : <></>}

{multiPage && {progress}% }

victorgcramos · 2021-06-28T20:52:20Z

src/containers/Comments/Download/DownloadCommentsTimestamps.jsx

+    <div>
+      <DownloadJSON
+        label={"Comments Timestamps"}
+        fileName={`${recordToken}-comments-timestamps`}
+        content={timestamps}
+      />
+    </div>


Wrapping this inside a <div> element creates an odd alignment on the download progress. See:

lukebp · 2021-06-29T18:01:55Z

A ticketvote/v1/details request is being made when you load the proposal details page. This is a expensive route and should only be retrieved when the votes bundle is being downloaded.

This is an issue on master. It was not introduce by this pull request. Please disregard this comment.

lukebp · 2021-06-29T20:35:32Z

Two pre-existing bugs were discovered during review of this pull request. #2460 and #2461.

This pull request does not need to fix 2460, but it should assume that the vote details call is being removed and should not rely on the vote details being available prior to download. 2461 should be fixed in this pull request.

To accomplish both of these it should:

Retrieve the vote details when the Votes Bundle button is clicked.
Retrieve the vote results when the Votes Bundle button is clicked.
Populate the Votes Bundle correctly with the retrieved data.

…ult) instead of cache data

victorgcramos

Progress percentage is till with a wrong alignment. It should be vertically centralized with the progress circle.

victorgcramos · 2021-07-05T19:09:39Z

src/components/RecordWrapper/RecordWrapper.jsx

    auths: voteSummary?.details?.auths,
    details: voteSummary?.details?.details,
    votes: voteSummary?.votes,
    serverpublickey
  };
-  return <DownloadJSON fileName={fileName} label={label} content={bundle} />;
+  return <DownloadJSON fileName={fileName} label={label} content={bundle} />;*/


victorgcramos · 2021-07-05T19:09:58Z

src/components/RecordWrapper/RecordWrapper.jsx

+  /*const bundle = {
    auths: voteSummary?.details?.auths,
    details: voteSummary?.details?.details,
    votes: voteSummary?.votes,
    serverpublickey
  };


…nt, remove decimal of progress percentage

lukebp

The UX is looking good. See the comment about the server public key though.

src/actions/api.js

lukebp

The votes bundle JSON has been changed from details to detail. You can't make changes like this. It breaks other software that expects details. Please change it back.

vibros68 · 2021-07-06T17:35:01Z

I am sorry. It is fixed now

lukebp

tACK

This looks good from a UX standpoint. Verification using politeiaverify works as expected with all bundle downloads.

@tiagoalvesdulce and @victorgcramos will need to approve the code.

tiagoalvesdulce

LGTM

victorgcramos

LGTM!

adjust download bundle: show loading wheel while the data is being fe…

b334a68

…tched; show progress for paginated downloads only when there are more than 1 page

fix Download Comments Timestamp progress is wrong value

25811aa

lukebp requested changes Jun 22, 2021

View reviewed changes

victorgcramos requested changes Jun 28, 2021

View reviewed changes

vibros68 added 3 commits June 30, 2021 23:09

fix javascript compact syntax, fix html redundant tag

7093c8e

cache

30987c5

Votes Bundle download data from the server(votes details and vote res…

6f12a0a

…ult) instead of cache data

vibros68 force-pushed the 2438_Download_bundle_UX_improvement branch from 5d613f8 to 6f12a0a Compare July 5, 2021 02:22

vibros68 requested review from lukebp and victorgcramos July 5, 2021 04:26

victorgcramos requested changes Jul 5, 2021

View reviewed changes

remove unnecessary comments, fix Progress percentage is wrong alighme…

8da4581

…nt, remove decimal of progress percentage

lukebp requested changes Jul 6, 2021

View reviewed changes

src/actions/api.js Show resolved Hide resolved

add serverpublickey to the votesBundle download

6b94a03

lukebp requested changes Jul 6, 2021

View reviewed changes

change name of votesbulde back to details from detail

cc0ca18

lukebp approved these changes Jul 6, 2021

View reviewed changes

tiagoalvesdulce approved these changes Jul 8, 2021

View reviewed changes

victorgcramos approved these changes Jul 9, 2021

View reviewed changes

tiagoalvesdulce merged commit 539b697 into decred:master Jul 9, 2021

tiagoalvesdulce added the pi-not-deployed label Jul 9, 2021

vibros68 mentioned this pull request Sep 25, 2021

Testing v1.2.0 release. #2597

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Download bundle UX improvement #2453

Download bundle UX improvement #2453

vibros68 commented Jun 22, 2021 •

edited

Loading

lukebp commented Jun 22, 2021 •

edited

Loading

vibros68 commented Jun 22, 2021 •

edited

Loading

lukebp left a comment •

edited

Loading

lukebp Jun 22, 2021

victorgcramos left a comment

victorgcramos Jun 28, 2021

vibros68 Jun 30, 2021

victorgcramos Jun 28, 2021

victorgcramos Jun 28, 2021

lukebp commented Jun 29, 2021

lukebp commented Jun 29, 2021

victorgcramos left a comment

victorgcramos Jul 5, 2021

victorgcramos Jul 5, 2021

lukebp left a comment

lukebp left a comment

vibros68 commented Jul 6, 2021

lukebp left a comment

tiagoalvesdulce left a comment

victorgcramos left a comment

	{multiPage ? <span style={{ marginRight: 10 }}>{progress}%</span> : <></>}
	{multiPage && <span style={{ marginRight: 10 }}>{progress}%</span>}

Download bundle UX improvement #2453

Download bundle UX improvement #2453

Conversation

vibros68 commented Jun 22, 2021 • edited Loading

Description

lukebp commented Jun 22, 2021 • edited Loading

vibros68 commented Jun 22, 2021 • edited Loading

lukebp left a comment • edited Loading

Choose a reason for hiding this comment

lukebp Jun 22, 2021

Choose a reason for hiding this comment

victorgcramos left a comment

Choose a reason for hiding this comment

victorgcramos Jun 28, 2021

Choose a reason for hiding this comment

vibros68 Jun 30, 2021

Choose a reason for hiding this comment

victorgcramos Jun 28, 2021

Choose a reason for hiding this comment

victorgcramos Jun 28, 2021

Choose a reason for hiding this comment

lukebp commented Jun 29, 2021

lukebp commented Jun 29, 2021

victorgcramos left a comment

Choose a reason for hiding this comment

victorgcramos Jul 5, 2021

Choose a reason for hiding this comment

victorgcramos Jul 5, 2021

Choose a reason for hiding this comment

lukebp left a comment

Choose a reason for hiding this comment

lukebp left a comment

Choose a reason for hiding this comment

vibros68 commented Jul 6, 2021

lukebp left a comment

Choose a reason for hiding this comment

tiagoalvesdulce left a comment

Choose a reason for hiding this comment

victorgcramos left a comment

Choose a reason for hiding this comment

vibros68 commented Jun 22, 2021 •

edited

Loading

lukebp commented Jun 22, 2021 •

edited

Loading

vibros68 commented Jun 22, 2021 •

edited

Loading

lukebp left a comment •

edited

Loading