-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Resource upload field snippet #6227
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
Conversation
|
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.
@wardi this looks good so far, I added some comments and questions
To your behavior change comments:
The upload button no longer immediately opens your browser file dialog, a second click on the revealed file upload widget is now required.
See comment here. I like to get the file picker after the first click
clearing existing uploads keeps the file upload section selected instead of resetting to the "Data: [upload] [link]" menu. A second click on the "Remove" button will return to the menu. This could be changed to old behavior with a couple lines of JS if reviewer thinks appropriate.
I don't think that's a big deal. Actually if you have an existing upload you are more likely to replace with another upload than a URL so it makes sense.
Remove buttons are now positioned to the right of the field labels instead of within the edit box for uploads and links. This was the easier implementation and I think it improves usability to have a margin between different interactive elements
I personally like the "Remove" buttons inline with the input box. I think it makes clear what you are clearing
@@ -0,0 +1,114 @@ | |||
{# |
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.
This snippet is not the easiest to read but is definitely easier to follow than the old JS module one.
Just to see if I'm reading it right is this how you would add your own button (in your own resource_upload_field.html
)?
{% ckan_extends %}
{% block url_type_select %}
{{ super() }}
{# Add the button here #}
{% endblock %}
{% block url_type_fields %}
{{ super() }}
{# Add the actual field here #}
{% endblock %}
And with more detail:
{% ckan_extends %}
{% block url_type_select %}
{{ super() }}
{# Add the button here #}
<button type="button" class="btn btn-default" id="resource-import-button"
title="{{ _('Some other way of adding data') }}"
onclick="
document.getElementById('resource-import-link').checked = true;
document.getElementById('field-resource-import').focus();
"><i class="fa fa-globe"></i>{{ _('Import Data') }}</button>
{% endblock %}
{% block url_type_fields %}
{{ super() }}
{# Add the actual field here #}
{% is_import = data.get('import') %}
<input type="radio" id="resource-url-import" name="url_type" value="import" {{
'checked' if is_import else '' }}>
<div class="select-type">
{% block link_controls %}
{{ remove_button(
js="$('#field-resource-import').val('')") }}
{{ form.input(
'import',
label= _('Import URL'),
id='field-resource-import',
type='url',
placeholder=placeholder,
value='' if is_upload or is_url else data.get('import'),
error=errors.get('import'),
classes=['control-full']) }}
{% endblock %}
</div>
{% endblock %}
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.
I provided an example for adding a button above #6227 (comment)
Co-authored-by: Adrià Mercader <amercadero@gmail.com>
Co-authored-by: Adrià Mercader <amercadero@gmail.com>
Co-authored-by: Adrià Mercader <amercadero@gmail.com>
Re: the remove button position change, it was also tricky to write the CSS to have the remove button overlap with the input box, and overlapping wouldn't make sense for all url_type inputs so it will make the code more complicated. Can you think of a way to manage this per url_type? raw css inside the templates? |
These CSS tweaks bring the button aligned as we have before: diff --git a/ckan/public/base/scss/_forms.scss b/ckan/public/base/scss/_forms.scss
index 415278715..788a5cd69 100644
--- a/ckan/public/base/scss/_forms.scss
+++ b/ckan/public/base/scss/_forms.scss
@@ -779,10 +779,10 @@ select[data-module="autocomplete"] {
}
.btn-remove-url {
- // "Remove" buttons float right of the select-type div labels
position: absolute;
margin-right: 0;
- top: 0;
+ margin-top: 2px;
+ top: $grid-gutter-width;
right: $grid-gutter-width / 6;
padding: 0 12px;
border-radius: 100px;
I think that the aligned buttons make sense for all input field based url_types and will keep the existing UI, and if needed extensions can either override the CSS or not include the What do you think? |
I see that with these changes resources that are created with Links have a I assume that this will only happen automatically if users are creating the resource from the UI (because the form field is sent). If external resources are created via the API the |
@amercader these should be fixed now |
Thanks @wardi! merging now |
Copy the form elements from upstream CKAN. We use the form fields in CKAN 2.10, which were refactored to simplify them and drop the infamous image-upload.js module in ckan/ckan#6227. The markup is clearly separated in three blocks: 1. Upload/Link buttons 2. File input + Remove button 3. URL input + Remove button The style classes have been prefixed to avoid clashes with the other `image-upload` field we have in the form (the schema one, which will eventually be replaced): `.resource-upload-field` -> `.ckan-resource-upload-field` `.btn-remove-url` -> `.btn-remove-url-upload` TODO: * Remove inline JS * Autopopulate name * Hook into upload logic
Fixes #6226
Proposed fixes:
Create extensible resource upload snippet with reduced JS, separate from the image upload macro. Allows easily adding new buttons to the the "Data: [upload] [link]" menu part of the resource form.
Behavior changes
That this PR includes changes from #6218 which should be reviewed first
Features: