Arvados

Updates at b8dccb1fa - branch 12479-wb-structured-vocabulary

• Remote file loading code still pending, just using a mocked vocabulary for tests
• When having selected a tag that renders a SelectField and changing to other that does the same, the SelectField doesn't get refreshed with the new options
• Visual issues: the input text fields don't have the correct vertical alignment/size, Cancel & Save buttons separation, options on the select side by side instead of one below the other.

Updates at 97b13b759

Added loading of remote vocabulary w/fallback.

target uuid should be passed in from the data-mount-mithril element, not parsed from page URL (I thought I had added this already but it seems to be on an unmerged branch spike-wb-browse-collection)

diff --git a/apps/workbench/app/assets/javascripts/mithril_mount.js b/apps/workbench/app/assets/javascripts/mithril_mount.js
index f4689b51d..7995ffea6 100644
--- a/apps/workbench/app/assets/javascripts/mithril_mount.js
+++ b/apps/workbench/app/assets/javascripts/mithril_mount.js
@@ -4,6 +4,7 @@

 $(document).on('ready arv:pane:loaded', function() {
     $('[data-mount-mithril]').each(function() {
-        m.mount(this, window[$(this).data('mount-mithril')])
+        var data = $(this).data()
+        m.mount(this, {view: function () {return m(window[data.mountMithril], data)}})
     })
 })

...then you can get vnode.attrs.uuid=="baz" by doing

<div data-mount-mithril="foobar" target_uuid="baz"></div>

"git diff" is showing me some red-highlighted trailing whitespace

"selectize" should be imported via npm, not copypasted into the source tree

Should we be adding the integer field now vs. some point in the future? Constraints like this can act weird when the current tag values don't comply with the vocabulary specs, e.g., it's fine if you leave it alone, but once you start editing it, you can't revert to the previous value without abandoning and doing a page-reload. Of course, a feature with limitations can be better than no feature at all.

In sessiondb, loadLocal() seems like it could just as well use Object.values() since it never uses the key

I don't think we need vnode.state.tagTable if it's always TagTable

Seems like Vocabulary class could be replaced by a stream

vnode.state.vocabulary = m.stream({"strict":false,...})
m.request('/vocabulary.json').then(vnode.state.vocabulary)

Tags class also feels too complicated -- could it be just a plain array [{"name":m.stream("foo"), "value":m.stream("bar")}, ...]?

As for fixing/simplifying the synchronization issues: it might help to have just one kind of "select" widget that accepts a list of options and a value stream, and updates the value when the user chooses/types something.

var TagEditorRow = {
    view: function(vnode) {
        return m('tr', [
            m(SelectOrAutocomplete, {
                options: vnode.attrs.vocabulary.keys(),
                value: vnode.attrs.name,
            }),
            m(SelectOrAutocomplete, {
                options: vnode.attrs.vocabulary[vnode.attrs.name()].options,
                value: vnode.attrs.value,
            }),
        ])
    },
}

var TagEditorTable = {
    oninit: function(vnode) {
        vnode.state.tags = vnode.attrs.properties.keys().map(function(key) {
            return {
                name: m.stream(key),
                value: m.stream(vnode.attrs.properties[key]),
            }
        })
    },
    view: function(vnode) {
        return m('table', vnode.state.tags.map(function(tag, idx) {
            return m(TagEditorRow, {
                key: idx,
                name: tag.name,
                value: tag.value,
                vocabulary: vnode.attrs.vocabulary,
            })
        }))
    },
}

Then SelectOrAutocomplete can be a generic thing that doesn't need to know anything about tags -- it just translates the lifecycle into the appropriate selectize calls. I imagine it shouldn't be too hard to tell selectize to update, but even if we need to do shenanigans like "notice whether attrs.options has changed, and destroy/recreate the selectize jquery stuff", that can all be encapsulated in the component. From the outside it will just behave like a nice component.

(Passing {key: tag.name()} might be an easy way to force mithril to build a new row instead of using an existing one. But I think things generally work better if we can update the row, when (from the user's perspective) the row is being edited, not replaced.)

What do we want to happen when the user types the same tag key into multiple rows?

Should use "select" param when getting current tags, mostly to avoid loading collection manifest_text unnecessarily

Might be a little neater to do vnode.state.editMode && m("div", ...) rather than vnode.state.editMode ? m("div", ...) : ""

Using a button tag + btn classes for the "delete" button would probably improve the layout a bit, and avoid the need for "cursor: pointer" style

Instead of setting a "dirty" flag in SelectOrAutocomplete, we could attach a "set dirty" flag in TagEditorApp.oninit after setting up vnode.state.tags ... and this could reduce the amount of redrawing a bit too, since we really don't need to redraw when values change:

vnode.state.dirty = m.stream(null)
vnode.state.tags.map(function(tag) {
  tag.name.map(m.redraw)
  tag.name.map(vnode.state.dirty)
  tag.value.map(vnode.state.dirty)
})

("add tag" would also need something to avoid going out of sync)

• instead of an "add new tag" button it might be trivial to implement "add a new tag if the last row isn't empty"

Updates at 47e59a35d5ed9b2cdb052894d741972324058505
Test run (failed): https://ci.curoverse.com/job/developer-run-tests/541/

• Save button disables when changes are synced to the server. Moved to the top of the table.
• "Delete tag" button re-styled
• When making a request, ask the API server for only the collection's properties.
• When no tags, show an empty table with a message

Ongoing: fix test failures

At 47e59a35d, in case this differs from your results -- testing on chrome, I find the select-or-type boxes don't quite behave. After adding a new row, clicking "new tag" shows a dropdown and puts a text cursor next to "new tag" so it looks like I can type. But my typing is ignored, except Backspace, which clears the field, and (presumably because we rekey the row) closes the dropdown and (not sure why) resets the "Save" button as if everything has been saved (it hasn't).

Possibly unrelated -- can we prevent the "new value" entry from appearing in the value dropdown (try: add tag; choose a tag from the tag dropdown; choose a value from the value dropdown; open the value dropdown again → "new value" is one of the options, but shouldn't be) ... If the idea is to remind the user that it's possible to type an arbitrary value, could we call it "(other)", make sure it's always at the bottom (or top) of the list, and clear the text box when it's selected instead of putting the placeholder text there?

On that note, it would be great if we could use the text input "placeholder" feature to make the words "new tag" appear in the text box when nothing has been entered/selected, but without making the user backspace over it before entering real text.

Updates at 407033514

Addressed the issues described above.

Updates at 5dc1784a0
Test run: https://ci.curoverse.com/job/developer-run-tests/542/

• Syntax error (only detectable by our test environment) fixed.
• Fixed conditional on session_db

Updates at 2ada2ce83
Tests: https://ci.curoverse.com/job/developer-run-tests/545/

Did some cleanup to make the tests suite pass OK.
New tag editor (mithril apps test suite) tests are missing.

Updates at 7d0d61f43

As requested by Tom Morris, allow a selectable tag to be overridable by default. Also, renamed the overridable property to strict for consistency. Now both toplevel and type-specific strict properties are assumed false if not set to true.

Remove example vocabulary.json (or rename to vocabulary-example.json?)

The "type" field seems superfluous, suggest removing it (the example could say "text tag": {} and it would work the same way, right?).

Suggest format

{
    "strict": false,
    "tags": {
        "fruit": {
            "values": ["pineapple", "tomato", "orange", "banana"],
            "strict": true
        },
        "animal": {
            "values": ["human", "dog", "elephant", "eagle"],
            "strict": false
        }
    }
}

• New tag
• Click tag name selector
• Type some chars
• Press Tab key → cursor disappears
• Press Tab key 2× more → cursor arrives in value selector

Why compare strict === true instead of just strict? (It seems like interpreting strict=="true" as false is no less surprising than interpreting strict=="false" as true -- if we're going to silently truthify all values I'm inclined to do it the normal javascript way instead of making a special rule.)

Instead of !(vnode.state.dirty() === false), could we just say vnode.state.dirty(), and make sure name/value updates always set dirty to true (never "" etc) by saying tag.name.map(vnode.state.dirty.bind(null, true))? (As an aside, if you really need !(foo===bar) you can just say foo!==bar.)

Instead of {border:1} you can use the bootstrap style ".table-bordered" (personally I think there are already enough horizontal and vertical lines all over the place that the table borders are unnecessary, but w/e)

2× uses of .pull-left seem superfluous

There are a few trailing-whitespace errors indicated by git-diff, and indentation is off in a few places like this. I find putting children in an array can help keep indentation regular, e.g., m("td", [ instead of just m("td", here:

            // Tag name
            m("td",
            vnode.attrs.editMode ?
            m("div", {key: 'name-'+vnode.attrs.name()},[m(SelectOrAutocomplete, {

Updates at 7c7dae7f8f4ec4e33acf9302152907547200023d

• Removed the "add new tag" button, a new empty row is added on demand (requested at the end of note-10)
• Fixed widget auto-focus when adding new tag names and values
• Fixed conditional usages
• Simplified vocabulary definition format
• Renamed example vocabulary file

Pending: last 2 points of note-18, but I wanted to check if it is looking better now.

Updates at f61d8bca07c8792dfd6216119ee63c573810963a

Removed table borders, fixed indentation.

• The worst part is that I can't just click a field that says "foo" and type "bar" to change it to "foobar". When I click "foo", a text cursor appears after "foo", but typing is ignored. Backspace, instead of giving me "fo" like I'd expect, clears the entire field. I can't even figure out how to copy the existing text with either the mouse or the keyboard -- my only option is to retype the whole thing by hand. Is this browser-dependent, or another dirty-cache problem with my setup, or just what we get with selectize, or....?
• It's a bit odd that the autocomplete entries grow to include the previous values I've typed. This seems to make it easy for a previously saved custom value (or something that was mistyped by the user in this session) is indistinguishable from the official options. Real users should tell us whether this is a bug or a feature though; I don't think we should try too hard to second-guess them here.

Looks better to me with fewer table borders. Eliminating the horizontal lines would be even better IMO, but doesn't seem like the highest priority.

Auto-adding rows feels easier, good change IMO.

Updates on 90dd1c90f

• When no vocabulary is in use, just use a simple text input widget as before, to avoid confusion.
• Removed all horizontal lines on the tags table
• Tweaked selectize widget to enhance usability: don't open the dropdown menu when focusing, don't persist a custom option when deslecting it, among other minor details.

Lucas Di Pentima wrote:

• When no vocabulary is in use, just use a simple text input widget as before, to avoid confusion.

Yes, this is definitely better for the no-vocab case.

• Removed all horizontal lines on the tags table

Looks way better, thanks.

• Tweaked selectize widget to enhance usability: don't open the dropdown menu when focusing, don't persist a custom option when deslecting it, among other minor details.

Much better.

I still can't figure out how to edit a tag or value, though. Regardless of whether it was chosen from a list or typed in manually, once I lose focus and return to it, the only way I can modify the text is by typing Backspace, which clears the field completely. I can't insert or remove individual characters or even select the existing text in order to copy-paste it. This seems kinda frustrating when fixing typos. Any ideas?

Updates at 7ed41dc01

I think I solved the value editing problem.

OK, editing existing stuff works!

• Tabbing behaves strangely. I need to hit "tab" twice to get from a tag key to a tag value input. (Where does it go in between??) Tabbing from key to value on the last non-empty row seems to be impossible: first tab unfocuses, second tab brings focus back to the key input. I'm not sure this is better than the previous tabbing behavior. Looking at the code, it seems like we're setting focus during each render cycle based on the content of the inputs, without regard to where focus was before, which seems kinda wrong. I'm guessing the whole focus adventure started because we lose focus order when deleting the focused element. If that's right, we might just need to drop the "key" attr on the tag name div, or use the same key every time -- after all, it's only the "value" input, not the "name" input, that needs to be recreated when name() changes in order to update its options and therefore needs to change its key.
• When I choose a strict=true tag key like "fruit", then click the value box, I just get a text-entry field with a cursor and a "new value" prompt (the dropdown doesn't open yet). I can type some text, but it just disappears when I unfocus via tab or clicking elsewhere, unless I happen to type one of the listed values and hit Enter before tabbing away. Depending on what I type (e.g., "fig", since none of the example fruits have the letter f) the dropdown doesn't necessarily appear at all during this process, which makes it especially confusing when the entered text disappears. Can we make it so strict=true means the dropdown opens right away and I'm not encouraged/able to type text?

Tom Clegg wrote:

• When I choose a strict=true tag key like "fruit", then click the value box, I just get a text-entry field with a cursor and a "new value" prompt (the dropdown doesn't open yet). I can type some text, but it just disappears when I unfocus via tab or clicking elsewhere, unless I happen to type one of the listed values and hit Enter before tabbing away. Depending on what I type (e.g., "fig", since none of the example fruits have the letter f) the dropdown doesn't necessarily appear at all during this process, which makes it especially confusing when the entered text disappears. Can we make it so strict=true means the dropdown opens right away and I'm not encouraged/able to type text?

Updates at e5b7fb125 enhances the user experience by showing a suitable placeholder message when the user can and cannot add custom values. On those cases when the user just has to select one option, the dropdown list is opened when focusing and when typing something that doesn't match any option available, a "No results found" message is rendered instead.

Selectize also has an option to select the highlighted/typed value when unfocusing, but this disabled for the moment until I figure out how to solve the tab navigation issues from your first point on note-25.

I can now edit an existing entry (yay!) and I'm not taunted by text entry fields on strict options (yay!) but this combination seems to have broken strict options a bit: if I click/tab a strict-mode value and then click/tab away from it without editing, the existing entry gets cleared.

"No results found" seems like a nice UI improvement but I can't find any license info and I'm not sure how important it is. Maybe we should just wait for it to be included in the next selectize release? I feel like at this stage of the adventure we should focus (ha ha) on getting the basic functionality to work correctly so we can merge, rather than adding new parts.

Updates at 3fc4aaee466529bd8ed7fad664d2eb61b37d6864

Replaced selectize with awesomplete to fix some usability issues that I couldn't correct on selectize.

Updates at 5a82b3749

Re-added 'key' to the tag key div, because the lack of it produces an odd behavior when removing some existing tag using the trash button and then trying to add a new tag.

I see some code that claims to open the list of choices when focusing a blank input, but that doesn't work for me (on either the name or value input). The list doesn't open until I type a character. If I then backspace over it, the list stays open.

The autocomplete list maxes out at 10 entries, and it doesn't display any indication that there are more, let alone a way to see them without guessing what to type to bring them to the top. Maybe just increasing 10 to actual-number-of-choices would work -- it would be better to have a scrollbar, but even without one, at least none of the choices would be hidden.

Just as in note-25, when I choose a strict=true tag key like "fruit", then click the value box, I just get a text-entry field with a cursor and a "new value" prompt (the dropdown doesn't open yet). I can type some text, but it just disappears when I unfocus via tab or clicking elsewhere, unless I happen to type one of the listed values and hit Enter before tabbing away. Depending on what I type (e.g., "fig", since none of the example fruits have the letter f) the dropdown doesn't necessarily appear at all during this process, which makes it especially confusing when the entered text disappears.

I noticed it's not easy to make my browser start using an updated vocabulary.json after changing it on disk. Ctrl-Refresh on the tag editing page doesn't do it. I need to open vocabulary.json in a browser tab, and hit refresh there. We could hack around this by loading vocabulary.json?v=12345 where 12345 is int(timestamp / 5min), forcing a reload every ~5 minutes. Other thoughts? "Tell all your users to open vocabulary.json in a new tab and hit Refresh" would be rather unfortunate.

Tabbing works, though! :)

I've pushed 12479-wb-structured-vocabulary @ d8a7800b5ca3d50bcd62545711585681e2b9154b which seems to fix the focusing stuff.

Clearing a field (e.g., select-all + backspace) doesn't set the "dirty" flag. This is special-cased in appendTag, but the comment just explains what a !== '' comparison does, which isn't especially helpful. I'm guessing the purpose is to avoid setting the dirty flag when adding a new empty row. It will take a bit of work to do this correctly so I suggest we don't do it. Instead, just check editMode and append a blank row before setting dirty(false). That should offer a blank row without setting the dirty flag.

Haven't looked at the "open on focus" thing yet, aside from noticing that it still doesn't happen after a bunch of reloading.

I notice the tab refreshes a few seconds after saving, which is kind of weird because the "saved" feedback has already been given. I haven't confirmed but I'm guessing this is just the usual refresh-because-websocket thing. Should we disable auto-refresh on this tab? (Refresh-and-lose-edits-because-something-else-happened-elsewhere would probably be considered a bug too.)

• fixes strict mode (broken in d8a780)
• fix "open autocomplete when focusing an empty field" (awesomplete docs seem to agree with my browser behavior re evaluate(), but calling evaluate() and then open() seems to work)
• ensure dirty gets set even after clearing a field

After all this, tabbing seems to work normally, even saving whatever was typed/selected (unless it's invalid content on a strict-mode input). At least for chrome-on-linux. :\

More updates on 0cae64114

• Accept tags with empty values
• Avoid tags tab reloading
• Avoid sorting
• Show all (1 million max) options on a scrollable listing
• Clear vocabulary.json cache every 5 minutes as suggested by Tom (cool idea!)

LGTM thanks!