Bug 17929 - You can't edit indicators in the cataloging screen
Summary: You can't edit indicators in the cataloging screen
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major (vote)
Assignee: Josef Moravec
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 16239
Blocks: 15437
  Show dependency treegraph
 
Reported: 2017-01-18 02:20 UTC by Caitlin Goodger
Modified: 2017-02-07 21:19 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 17929 You can't edit indicators in the cataloging screen (2.64 KB, patch)
2017-01-18 20:44 UTC, Caitlin Goodger
Details | Diff | Splinter Review
Bug 17929 You can't edit indicators in the cataloging screen (1.20 KB, patch)
2017-01-18 23:30 UTC, Caitlin Goodger
Details | Diff | Splinter Review
Bug 17929 You can't edit indicators in the cataloging screen (1.20 KB, patch)
2017-01-18 23:34 UTC, Caitlin Goodger
Details | Diff | Splinter Review
Bug 17929 You can't edit indicators in the cataloging screen (1.25 KB, patch)
2017-01-20 12:50 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 17929 You can't edit indicators in the cataloging screen (1.26 KB, patch)
2017-01-20 12:51 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 17929 - You can't edit indicators in the cataloging screen (1.25 KB, patch)
2017-01-24 07:23 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17929 - You can't edit indicators in the cataloging screen (1.37 KB, patch)
2017-01-27 13:56 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Caitlin Goodger 2017-01-18 02:20:35 UTC
As the bug suggest you can no longer edit indicators in the cataloging screen. This used to work
Comment 1 Katrin Fischer 2017-01-18 02:23:21 UTC
I can confirm this.
Entering indicators in master is currently not possible. The cursor appears, but you are not able to enter anything.
Comment 2 Chris Cormack 2017-01-18 07:31:44 UTC
Turns out you can type, you just can't see what you are typing.

It is the indicator class in the css that is causing it.

For some reason it now doesn't like 1em as a width, it is too small. (At least in firefox)
It seems to work ok in chrome
Comment 3 Chris Cormack 2017-01-18 07:36:50 UTC
It is caused by bug 16239

So I am not sure how to fix it, but it will need to be fixed before release or you can't catalogue with firefox
Comment 4 Jonathan Druart 2017-01-18 08:12:05 UTC
Lowering the severity a bit.
Comment 5 Jonathan Druart 2017-01-18 08:17:20 UTC
Weird indeed. We could simply remove the width for .indicator, that does the trick (i.e. we see what we type).

Ccing Owen and Josef to get a better solution.
Comment 6 Josef Moravec 2017-01-18 09:41:25 UTC
It is caused by different box-sizing in bootstrap 3 (border-box)

We have to options:
1) remove width: 1em styling for indicators - the width calculation will fallback to size parameter of input tag

2) or change the box-sizing to content-box for indicators

a have no strong opinion on this, probably would go with 2) as it looks more similar to pre-16239 ages ;)
Comment 7 Caitlin Goodger 2017-01-18 20:44:22 UTC
Created attachment 59177 [details] [review]
Bug 17929 You can't edit indicators in the cataloging screen

Test Plan
1: Find a Bibiographical record and go to edit record
2: You won't be able to type into the indicators
3: Apply the patch
4: Refresh the page and now the boxes should be bigger so that you can
type into there.
Comment 8 Katrin Fischer 2017-01-18 22:57:36 UTC
Sorry, but there are some problems with this solution:

The patch removes the class 'indicator', but classes are not only used for styling, but also for targetting elements in Javascript. One negative side effect of removing the class is, that the authority linking plugin does not correctly work anymore (a white window remains, Javascript error indicates problem with filling indicators).

I think it would be a safer and nicer solution to keep the class, but change the CSS tied to it. I did some experimentation with the browser tools - maybe another solution to explore could be to change the width. I turned it up to 2 em and was able to enter indicators again. 

We will also need the same fix for the authority record editor - it shows the same bad symptoms right now.
Comment 9 Caitlin Goodger 2017-01-18 23:30:07 UTC
Created attachment 59195 [details] [review]
Bug 17929 You can't edit indicators in the cataloging screen

Test Plan
1: Find a Bibiographical record and go to edit record
2: You won't be able to type into the indicators
3: Apply the patch
4: Refresh the page and now the boxes should be bigger so that you can
type into there.

On my window you still can't see the indicator properly so you can
choose what looks better between the 2em and the 3em one. This is the
2em one
Comment 10 Caitlin Goodger 2017-01-18 23:34:18 UTC
Created attachment 59196 [details] [review]
Bug 17929 You can't edit indicators in the cataloging screen

Test Plan
1: Find a Bibiographical record and go to edit record
2: You won't be able to type into the indicators
3: Apply the patch
4: Refresh the page and now the boxes should be bigger so that you can
type into there.

I have made a patch for 2em and 3em and this is the 3em one because the
2em is still too small on my screen. Choose which one works better for
you
Comment 11 Mirko Tietgen 2017-01-20 12:50:21 UTC
Created attachment 59355 [details] [review]
Bug 17929 You can't edit indicators in the cataloging screen

Test Plan
1: Find a Bibiographical record and go to edit record
2: You won't be able to type into the indicators
3: Apply the patch
4: Refresh the page and now the boxes should be bigger so that you can
type into there.

On my window you still can't see the indicator properly so you can
choose what looks better between the 2em and the 3em one. This is the
2em one

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Comment 12 Mirko Tietgen 2017-01-20 12:51:34 UTC
Created attachment 59356 [details] [review]
Bug 17929 You can't edit indicators in the cataloging screen

Test Plan
1: Find a Bibiographical record and go to edit record
2: You won't be able to type into the indicators
3: Apply the patch
4: Refresh the page and now the boxes should be bigger so that you can
type into there.

I have made a patch for 2em and 3em and this is the 3em one because the
2em is still too small on my screen. Choose which one works better for
you

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Comment 13 Mirko Tietgen 2017-01-20 12:52:29 UTC
Both patches make indicators work. The second one works better for me.
Comment 14 Jonathan Druart 2017-01-23 08:40:07 UTC
Please see comment 6 for a better solution.
Comment 15 Chris Cormack 2017-01-23 21:07:40 UTC
(In reply to Jonathan Druart from comment #14)
> Please see comment 6 for a better solution.

Can you explain more please Jonathan, they tried changing from indicator ot content-box and this failed.
Comment 16 Katrin Fischer 2017-01-23 21:09:19 UTC
I think the trick might be to add content-box without removing indicator. And just remove the styling applied to indicator from the CSS file.
Comment 17 Josef Moravec 2017-01-24 07:23:19 UTC
Created attachment 59487 [details] [review]
Bug 17929 - You can't edit indicators in the cataloging screen

Test plan:
0. Do not apply the patch
1. Edit a biblio record, note you can't edit an indicator (in fact you edit it, but can't see the value)
2. Edit an authority record, note you can't edit an indicator (in fact you edit it, but can't see the value)
3. Apply the patch, you may need clear the browser cache (in Firefox Ctrl+F5 is often enough)
4. Repeat steps 1 and 2 - but now you will be able to edit the indicator
Comment 18 Josef Moravec 2017-01-24 07:25:06 UTC
(In reply to Katrin Fischer from comment #16)
> I think the trick might be to add content-box without removing indicator.
> And just remove the styling applied to indicator from the CSS file.

That's what I meant too... Just edit css file..
Comment 19 Chris Cormack 2017-01-24 07:26:51 UTC
Seems a shame to take a patch off a high school student. But OK
Comment 20 Josef Moravec 2017-01-24 07:35:19 UTC
(In reply to Chris Cormack from comment #19)
> Seems a shame to take a patch off a high school student. But OK

Sorry for that, that was the reason, why I didn't do it before... I am a bit impatiant :( And of course I am feeling responsible for this bug, as it is introduced by my patchset on bug 16239...
Comment 21 J Schmidt 2017-01-26 21:24:12 UTC
Worked in sandbox ByWater on 1/26/17 (Chrome): Steps taken: 
1.) Searched in the staff client to locate a biblio record.
2.) Clicked on Edit record (to edit biblio)
3.) Defaults to the basic editor. 
4.) I click on indicators next to several MARC fields (243, 250, 710), I was able to edit and delete both the first and/or second indicators and successfully save without error. I was able to see the edits/changes. 
5.) Went back into edit biblio screen. Still in basic editing mode. I went into 245 field and noticed subfields can click on. I choose 245$h and changed that to a comma and was able to save it. This means there is no validation on retaining correct subfields but perhaps that can be set in the framework itself. 
6.) Clicking on the ? next to a MARC field correctly takes user to the MARC 21 format web page. (Works as should).
7.) Went to 100 field, entered in subfield a and then clicked on the icon to perform authority search to locate a personal name authority to link to subfield 9. Successfully got authority search dialog box to open up and perform searches. 
8.) Located a personal name authority Campbell-Smith, Duncan. Chose it. 
9.) 100$9 now populated with the authority record number and correct $a personal name. Saved without error. 
10.) Went into Koha admin > Labs, changed the preference to enable the advanced cataloging editor. Save.
11.) Went back into Edit biblio. Changed to Advanced editor. 
12.) Went into MARC 500 to edit indicators. I could edit and see numbers. Success. I had to tread carefully not to overwrite the field data itself. Upon save the advanced editor validates and tells you which tags are mandatory... Much improved.
13.) I put in bad indicators for the 300 field and it gave me invalid. Good. I had to do underline for blank which was not intuitive but me typing those in manually worked and the editor was able to detect error of incorrect indicator so far as I could tell.
14.) 003 is not familiar to me so I entered in DDC and Record saved successfully. 
15.) Went to 650 to try and add in CTRL+D 9 for authority record number...but no way to access authority search dialog box from this screen. 

I would say this is working well. The advanced editor still needs to allow macros that stick overtime and are tied to login so as not to lose them. Keyboard shortcuts or macros needed for fixed fields such as 007, 008, 006 especially.
Comment 22 Jonathan Druart 2017-01-27 13:56:39 UTC
Created attachment 59620 [details] [review]
Bug 17929 - You can't edit indicators in the cataloging screen

Test plan:
0. Do not apply the patch
1. Edit a biblio record, note you can't edit an indicator (in fact you edit it, but can't see the value)
2. Edit an authority record, note you can't edit an indicator (in fact you edit it, but can't see the value)
3. Apply the patch, you may need clear the browser cache (in Firefox Ctrl+F5 is often enough)
4. Repeat steps 1 and 2 - but now you will be able to edit the indicator

Signed-off-by: J Schmidt <jschmidt@switchinc.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 23 Kyle M Hall 2017-02-07 17:53:36 UTC
Pushed to master for 17.05, thanks Josef!
Comment 24 Katrin Fischer 2017-02-07 21:19:37 UTC
Bug 16239 is not in 16.11.x - patch is not needed there.