Angular MARC Editor Part 2 -- Enriched Editor

Bug #1852782 reported by Bill Erickson
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Wishlist
Unassigned

Bug Description

Wishlist circa Evergreen 3.4

The Angular MARC editor should offer enriched editing support in addition to its flat-text editor, consistent w/ AngJS and XUL editors.

I have started putting some pieces together in a working branch:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lpxxx-ang-marc-enriched-editor

I'll push a renamed branch soon. At present, it just has the fixed-field grid along the top.

My assumption is we want to do a more or less direct port of the AngularJS implementation. However, if we need to make changes, improvements, etc. please add suggestions here.

Revision history for this message
Christine Morgan (cmorgan-z) wrote :

Bill,
Have a look at bug 1735568, which makes some style improvements, as well as fixing some bugs.

Revision history for this message
Bill Erickson (berick) wrote :

Thanks for the heads up, Christine. I'll will take a look.

Revision history for this message
Bill Erickson (berick) wrote :

Hi All, I have a question about MARC editor undo/redo functionality.

In the AngularJS client, each character typed into an input (e.g. 245a subfield value) is tracked as a potential undo/redo action. This means if a user mistakenly types a long string of text into an input, a control-z action is required once per character to undo the typing. If they then decide to redo, the same number of control-y actions are required to recover the text.

By comparison, a typical browser input handles control-z / control-y a bit more intelligently. If you type a long string of text then undo, it will undo the entire action with a single command. Same for redo. The XUL MARC editor behaves the same in this regard. This seems more intuitive and efficient to me.

To take advantage of the stock browser behavior for undo/redo within a text input, though, we'd need a way to indicate undo/redo for other types of actions (add/remove field, subfield, etc.). We could accomplish that with a different set of key commands, e.g. control-shift-z / control-shift-y.

What do we think about separating the undo/redo actions into 2 types of actions:

ctrl-z / ctrl-y = undo/redo text input actions (with stock browser undo/redo behavior).

ctrl-shift-z / ctrl-shift-y = undo/redo field, subfield insert/delete actions.

Thanks

Revision history for this message
Elaine Hardy (ehardy) wrote :

I agree with having the undo/redo actions separated out into the two types.

Revision history for this message
Mike Rylander (mrylander) wrote :

IIRC, the reason I did it the way I did is that if you make a change to 245a, then 100a (without moving the cursor), then press ctrl-z a few times it only undoes the edits to 100a and does not "jump back" to undoing the 245a edits. I recall that specifically being an "issue" -- the whole record should be treated as a unit, and undo/redo should "just work". Of course, a side effect of the implementation was that /all/ edits of any type are kept in a single stack, which is nice when you look at it that way.

As you say, the tradeoff is that each edit is actually an edit, and has to be undone individually. Could we perhaps handle it a bit differently? If we record the field we're editing, and the edit amounts to just inserting or removing characters, just replace the most recent element on the stack so that the editing of a field is compacted down in the way that the native functionality acts? That would mean a single ctrl-z to undo all changes to a field when the context field stays the same.

Thoughts?

Revision history for this message
Bill Erickson (berick) wrote :

Thanks, Elaine. And thanks for the suggestion, Mike. I think that's a good compromise. Barring any other input, I'll implement the single-stack undo approach where text changes are treated as a unit instead of individual characters.

Revision history for this message
Jane Sandberg (sandbergja) wrote :

Wow, Bill! I just took a look at your branch today and it looks so cool!

A few notes:

* Almost no <input> fields have accessible labels. For the fixed fields, it seems pretty clear what to do: associate those inputs with their visible labels (like ELvl, Audn, DtSt, etc.) I'm not sure what kind of <aria-label> might be best for the variable length fields.

* It looks like this keyboard navigability issue still persists for the context menus: https://bugs.launchpad.net/evergreen/+bug/1836246

* The Undo/Redo functionality is really cool! I love it!

* When you navigate away from the MARC Editor in your browser, it's easy to lose unsaved work (similar to https://bugs.launchpad.net/evergreen/+bug/1538678)

* When you do the Shift+delete shortcut while a MARC tag is selected, it deletes the whole field. It might be safer to just not do anything on Shift+delete when a MARC tag is selected, so that it only ever deletes subfields, not whole fields.

Revision history for this message
Bill Erickson (berick) wrote :

Thanks for the testing and feedback, Jane!

I have pushed a (better named) branch, with a rebased and mostly squashed version of the code in progress. Included are commits to address the Shift+delete and navigate-away issues noted in Jane's previous comment.

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1852782-ang-marc-enriched-editor

Other issues and remaining TODOs in progress.

Revision history for this message
Bill Erickson (berick) wrote :

Fixes for keyboard navigation and aria labels pushed.

Revision history for this message
Bill Erickson (berick) wrote :

MARC editor code is now ready for review. I believe I have covered all of the features present in the AngJS editor. (Thanks to all devs who worked on the other MARC editors so I didn't have to figure all of that stuff out!). It's a pretty big set of features and functionality, though, so it would certainly benefit from thorough testing.

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1852782-ang-marc-enriched-editor

A couple of things to note:

1. This affects the experimental Angular catalog and the MARC edit option in the Vandelay queue manager interface.

2. Some of the business logic for managing authority browse and linking from the editor has been moved to server API's, i.e. a simple 'ng build' won't suffice for testing.

3. It does not affect the Create New MARC Record functionality or the bib record merge interface (from bib buckets).

tags: added: pullrequest
Changed in evergreen:
assignee: Bill Erickson (berick) → nobody
Revision history for this message
Jane Sandberg (sandbergja) wrote :

This looks cool, Bill!

1) Applying an authority works well, except that it doesn't add the $0 to the field.

2) I like how the authority linker includes the authority record ID. Would it be possible to turn that ID into a link to open that authority record in a MARC editor (probably in a modal)?

3) The buttons for the physical characteristics wizard and the authority linker need some sort of accessible label (probably via an aria-label or title)

4) There's an issue with the right-click context menu for the Ills fixed field, which also affects the AngularJS Marc editor (see bug 1857919)

5) Additionally, the fixed fields with more than one character don't seem to update at all when you click on the right-click context menu. For example, when I select a value for the Lang or Ctry fixed fields, it doesn't get applied.

6) The editor in Vandelay doesn't seem to be working for me. Every time I try to go over to the "Edit Record" tab in a Vandelay queue, I get this error in the console:

QueuedRecordComponent.html:22
NullInjectorError: StaticInjectorError(BaseModule)[MarcEditorComponent -> HoldingsService]:
  StaticInjectorError(Platform: core)[MarcEditorComponent -> HoldingsService]:
    NullInjectorError: No provider for HoldingsService!

Revision history for this message
Bill Erickson (berick) wrote : Re: [Bug 1852782] Re: Angular MARC Editor Part 2 -- Enriched Editor

On Mon, Dec 30, 2019 at 2:35 PM Jane Sandberg <email address hidden>
wrote:

> This looks cool, Bill!

> 1) Applying an authority works well, except that it doesn't add the $0
> to the field.
>

I was a little unclear on the intended behavior here. When applying a
heading, the AngJS editor only adds a $0 if one happens to already exist in
the heading field. (Note I surmised this by reading the code. I could not
verify in the UI since it was having some issues). Alternatively, the XUL
client gives you the option to apply the $0 or not.

If the expectation is the $0 is always added, which makes sense to me, then
I can change that.

>
> 2) I like how the authority linker includes the authority record ID.
> Would it be possible to turn that ID into a link to open that authority
> record in a MARC editor (probably in a modal)?
>

I could see that being handy. I propose we add a separate LP for that.

>
> 3) The buttons for the physical characteristics wizard and the authority
> linker need some sort of accessible label (probably via an aria-label or
> title)
>

Ah, I assumed the text content of the button itself served that purpose.
Should the aria-label/title just match the button text?

> 5) Additionally, the fixed fields with more than one character don't
> seem to update at all when you click on the right-click context menu.
> For example, when I select a value for the Lang or Ctry fixed fields, it
> doesn't get applied.
>

I'm unable to reproduce this. I tested Lang and Ctry and both resulted in
modified 008's. What browser are you testing?

>
> 6) The editor in Vandelay doesn't seem to be working for me. Every time
> I try to go over to the "Edit Record" tab in a Vandelay queue, I get
> this error in the console:
>
> QueuedRecordComponent.html:22
> NullInjectorError: StaticInjectorError(BaseModule)[MarcEditorComponent ->
> HoldingsService]:
> StaticInjectorError(Platform: core)[MarcEditorComponent ->
> HoldingsService]:
> NullInjectorError: No provider for HoldingsService!
>

Oops. Late addition to the code... I have pushed a fix for this.

Thanks for the testing and feedback, Jane!

-b

Revision history for this message
Jane Sandberg (sandbergja) wrote :

For #3, how about something like "Open physical characteristics wizard" and "Create authority link"? I think you're correct that a screen reader would read the ligature, but I don't think the ligatures themselves are descriptive enough.

For #1, the AngJS authority linker always adds $0s for me. My understanding was that without the $0, there is no row generated in authority.bib_linking, which means that the heading is not really subject to authority control at all.

Revision history for this message
Bill Erickson (berick) wrote :

#3 *facepalm* I was thinking of different buttons. I've pushed a patch to add titles for these. Thanks for the suggestions.

#1 I was able to get 1XX links to work in AngJS, but not 650's, etc. I'll try to reproduce in stock master. In any event, I do see now when applying a main entry heading it does update the $0, but not when applying a see from/also, which makes sense. I've pushed a patch to mimic this behavior in the new editor.

Thanks, Jane!

Revision history for this message
Bill Erickson (berick) wrote :

Branch rebased to master for Firefox fix fun.

Bill Erickson (berick)
tags: added: staffcatalog
Revision history for this message
Bill Erickson (berick) wrote :

Scratch my comment about opening a new LP for turning the authority IDs into links to open the authority records in their own editors. I've pushed a commit to the same branch to do this. From the commit:

Adds a new UI at /staff/cat/authority/edit/ for finding authority records by ID and editing authority records via the Angular MARC editor.

Modifies the "Cataloging" => "Retrieve Authority Record By ID" nav menu entries to point to the Angular version of the interface.

Augments the MARC edit authority linking dialog to turn authority ID's into links which open the authority record in its own MARC editor in a new tab.

Misc. MARC editor repairs related to loading authority records by ID.

Revision history for this message
Jennifer Pringle (jpringle-u) wrote :

In Chrome the text on menus that appear in the MARC Editor when you right click (fixed field grid, MARC field tags) are slightly fuzzy in comparison to the rest of the screen. In Firefox the text on the menus is crisp.

(Screenshot of Chrome attached.)

Revision history for this message
Jane Sandberg (sandbergja) wrote :

We discussed this at today's cataloging working group, and here is the feedback we got there.

We really like:
* Undo/Redo buttons in Enhanced MARC Editor
* The context menus for fixed fields, tags, etc.
* Nicely spaced and ample room
* Direct link to Authority Record

Some dislikes:
* Tabbing between subfields works differently in Firefox vs. Chrome. In Firefox, the tab key only tabs between delimiters, not the actual subfield data. We prefer the Chrome tabbing experience.
* The Delete and Save buttons are dangerously close to one another. To add to the danger, in other parts of Evergreen, Okay button tends to be on the left; here the delete button is on the left.

Bill Erickson (berick)
Changed in evergreen:
assignee: nobody → Bill Erickson (berick)
Revision history for this message
Bill Erickson (berick) wrote :

Fixes for the Dislikes noted by Jane pushed to the working branch.

I'm unable to reproduce the fuzzy characters issue noted by Jennifer on Linux, Mac, or Windows Chrome. Any help finding a pattern there is appreciated.

Changed in evergreen:
assignee: Bill Erickson (berick) → nobody
Revision history for this message
Jane Sandberg (sandbergja) wrote :

This has worked well in all my testing. Rebased and signed off branch here: user/sandbergja/lp1852782-ang-marc-enriched-editor

tags: added: signedoff
Bill Erickson (berick)
Changed in evergreen:
assignee: nobody → Bill Erickson (berick)
Revision history for this message
Bill Erickson (berick) wrote :

Thanks, Jane.

I did another round of testing and review. Merged to master.

Changed in evergreen:
status: New → Fix Committed
assignee: Bill Erickson (berick) → nobody
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.