Skip to content
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

Add clickthrough agreement for downloads #2947

Merged
merged 13 commits into from
Dec 5, 2023
Merged

Add clickthrough agreement for downloads #2947

merged 13 commits into from
Dec 5, 2023

Conversation

nellh
Copy link
Contributor

@nellh nellh commented Nov 27, 2023

This just needs text content to be ready for review.

The agreement is a floating banner on the bottom of the browser window. It disappears once you have agreed and enables download/view file links. Download pages redirect to the dataset page if the agreement has not been accepted. The agreement is persisted in local storage and shown again if the user is downloading from another browser or clears local storage.

image

Copy link

codecov bot commented Nov 27, 2023

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (39f6383) 64.89% compared to head (bd379d0) 65.08%.
Report is 4 commits behind head on master.

Files Patch % Lines
...pp/src/scripts/dataset/routes/download-dataset.tsx 28.57% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2947      +/-   ##
==========================================
+ Coverage   64.89%   65.08%   +0.19%     
==========================================
  Files         378      378              
  Lines       24230    24309      +79     
  Branches      868      884      +16     
==========================================
+ Hits        15723    15822      +99     
+ Misses       8507     8487      -20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@effigies
Copy link
Contributor

Can this be pushed to staging for an interactive view?

@nellh
Copy link
Contributor Author

nellh commented Nov 27, 2023

Can this be pushed to staging for an interactive view?

It's on staging now. I'm planning on making a couple minor fixes once I have the text as far as styling. The button gets pushed off the screen on mobile sometimes. Spacing can be a little off where the button gets cut off at some screen sizes. Let me know if you see other issues.

@effigies
Copy link
Contributor

When the button is visible, it can be pushed down a bit and get cropped by the bottom of the window:

image

Also, the "I Agree" seems like it should have a non-breaking space or for the text to be centered in the button.

Would it be possible to allow the text to scroll when the window gets too narrow? I assume the goal of not increasing the height is to prevent the page contents from being covered.

@effigies
Copy link
Contributor

Overall this works smoothly. Attempting to browse and search works fine, but attempting to view a file or a download page is blocked until I press the button, at which point the buttons/tab are instantly made visible.

@mgxd
Copy link
Contributor

mgxd commented Nov 28, 2023

This looks good as is, but what about instead of hiding the Download tab, it is always present but is un-clickable until the user agrees? I think that'll make it clear that the download is always there, but blocked.

…n accepted

This replaces the behavior of hiding the button
@nellh
Copy link
Contributor Author

nellh commented Dec 1, 2023

This looks good as is, but what about instead of hiding the Download tab, it is always present but is un-clickable until the user agrees? I think that'll make it clear that the download is always there, but blocked.

Thanks, I added this.

image

@nellh nellh marked this pull request as ready for review December 5, 2023 17:56
@nellh
Copy link
Contributor Author

nellh commented Dec 5, 2023

Text and styling for mobile is done now.

image

@nellh nellh merged commit a8fe2e1 into master Dec 5, 2023
7 checks passed
@nellh nellh deleted the feature/clickthrough branch December 5, 2023 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants