SPDX support + license selection improvements #1546
Conversation
fbacall
commented
Jul 25, 2023
fbacall
commented
- Adds SPDX licenses to the current supported set of licenses in SEEK (non-SPDX licenses are retained).
- Improves license selection to be searchable/filterable (using select2).
- Allows licenses to be provided by URI instead of just ID.
- Fixes Feature request: Licenses #114
- Fixes spdx licenses #655
- Helps support RO-Crate "license" field should be URI #456
Don't group select options if no recommended licenses provided
lib/seek/license.rb
Outdated
| licenses['notspecified']['title'] = I18n.t('null_license') | ||
| licenses['notspecified']['url'] = Seek::Help::HelpDictionary.instance.help_link(:null_license) |
There was a problem hiding this comment.
should be using NULL_LICENSE constant to get the key
app/helpers/license_helper.rb
Outdated
| opts[:data] ||= {} | ||
| opts[:data]['role'] ||= 'seek-license-select' | ||
| recommended = opts.delete(:recommended) | ||
| source = opts.delete(:source) || Seek::License.combined | ||
| if recommended | ||
| options = grouped_options_for_select(grouped_license_options(source.values, recommended), selected) | ||
| else | ||
| options = options_for_select(license_options(source.values), selected) | ||
| end | ||
| select_tag(name, options, opts) |
There was a problem hiding this comment.
would be good if this could be done via the objects_input helper, in the interest of keeping things self contained and easier to understand and maintain in the future. I can see the grouped options would be a problem here, but may be something useful elsewhere in the future. Did you give this any thought?
I'm a bit uncomfortable having different select2 implementations in various places, it was a pain and error prone when switching over from typeahead
There was a problem hiding this comment.
Could use objects_input, but I think it needs some tweaks to make it more appropriate for selecting single values.
There was a problem hiding this comment.
you can just include options[:limit]=1 for single values, which ends up setting select2's maximumSelectionLength when initializing - https://github.com/seek4science/seek/blob/seek-1.14/app/assets/javascripts/objects_input.js#L22C17-L22C44
Or perhaps I'm misunderstanding something, it's not clear to me how it is currently limiting the number of licenses selected
There was a problem hiding this comment.
if it's via the displayUrl function, might want to look at using select2 built in handlers, I added another review comment
There was a problem hiding this comment.
It's currently just a regular select tag (not a multiple select combobox) so it only allows 1 selection by default.
Limiting to 1 is a possibility, but then you have to clear the value with the X icon before you can choose a different license, instead of just directly selecting another option, which is a bit awkward
| displayUrl: function () { | ||
| var element = $j('option:selected', $j(this)); | ||
| var link = $j('#license-url'); | ||
| if (link.length) { | ||
| var block = link.parents('.license-url-block'); | ||
|
|
||
| if (element.data('url')) { | ||
| block.show(); | ||
| } else { | ||
| block.hide(); | ||
| } | ||
|
|
||
| link.attr('href', element.data('url')); | ||
| link.html(element.data('url')); | ||
| } |
There was a problem hiding this comment.
might want to look at https://select2.org/programmatic-control/retrieving-selections about retrieving selections, which also advices against reading the selected option. Probably also use select2:select as the event for when it has been selected.
* Allow single value mode for `objects_input` * Add ability to pass select options to `objects_input`
