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

Update uploader widget to follow ckan 2.10 design #2

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

aivuk
Copy link
Collaborator

@aivuk aivuk commented Mar 30, 2023

No description provided.

@aivuk aivuk added the enhancement New feature or request label Mar 30, 2023
@aivuk aivuk requested a review from amercader March 30, 2023 09:02
@aivuk aivuk self-assigned this Mar 30, 2023
@amercader
Copy link
Member

@aivuk this is getting better, but there's one piece missing, which is that the buttons and controls should be mutually exclusive, i.e.

  1. when you click a button, the relevant control appears (file or URL input) and the buttons disappear, and
  2. when you click remove, the input is cleared and hidden, and the buttons re-appear

This is shown in the screencast included in #1:

create_resource.mp4

@aivuk
Copy link
Collaborator Author

aivuk commented Mar 30, 2023

@amercader sorry, I missed that in your mockup. I did the change now, but I did need to add the field label "Data" to the ckan-upload component (before it was written on the ckanext-validation upload scheming template https://github.com/frictionlessdata/ckanext-validation/blob/feature/80-new-upload-widget/ckanext/validation/templates/scheming/form_snippets/ckan_uploader.html#L12) and removed from the ckanext-validation, otherwise it will not be simple to remove it from the js upload. What do you think?

This is how it looks now:

Screencast.from.03-30-2023.12.27.25.PM.webm

@aivuk
Copy link
Collaborator Author

aivuk commented Mar 30, 2023

I have some questions about the behaviour of the 'remove' button. With the upload we add automatically the filename and the schema when I a file is uploaded. What to do when I click remove, delete the content from the schema, mime type and name? Currently the old names keep in the fields.

@amercader
Copy link
Member

This is how it looks now:

Great, that'es exactly what I meant. The changes you mention make sense

What to do when I click remove, delete the content from the schema, mime type and name? Currently the old names keep in the fields.

The schema for sure. Check what happens in vanilla ckan with regards to name and format, but I think they are left there (which we might want to change tbh)

@amercader
Copy link
Member

@aivuk some quick comments :

  • I can't see any "File uploaded" message after a successful upload (or an error if there was one), is that missing? Or maybe is worth leaving the progress bar at 100% visible, basically just make sure that the user knows the file is updated at this point
    Screenshot 2023-02-28 at 13-36-36 Add resource - Test dataset 23222 - CKAN Catàleg Dades
  • Is the "Waiting for data schema detection" showing up? maybe it's too fast to come up, in this case is fine
  • There is an error with files too large (I think default max for CKAN is 100Mb). it's fine for the upload to fail but it should show an error
  • I think we could implement the same inferring logic if the user enters a URL (using a lose focus event or something)
  • Clicking the "Remove" button should clear the name, format and schema, if any

And one pending issue is the cleanup of unfinished resources. I think that should happen we clicking the "Remove" button or leaving the page. Both should ping an endpoint that calls "resource_delete" (I think there's a builtin CKAN core one, maybe we can just call that one)
For the "leaving the page" scenario we could leverage the onbeforeupload event, which could be regitered only if we have uploaded a file / added a URL. Can you make some tests to see if it works?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants