Page MenuHomePhabricator

Allow editing of fields
Closed, ResolvedPublic13 Story Points

Description

Motivation
As a user I want to be able to edit existing data entries of the termbox. This will increase data quality and the multilinguality of Wikidata.

Workflow
Given I'm on a page showing the termbox component in an editable state,
when clicking the edit button (pen icon)
then the termbox entries (labels, descriptions and alias(es)) of all shown languages will switch to edit mode.

Acceptance Criteria

  • There is an edit button on the top of the page (next to the label of the interface language)
  • When the user clicks on the edit button
    • all entries become editable, with label, description and alias texts turning into input fields Only labels and descriptions
    • The editing fields are correctly styled (see: T218572)
      • The input filed has the text heights plus 8px padding to each side
      • it is depicted by a 1px inside border in Base50. The 1px is taken off from the 8px padding described above so that we remain in a grid dividable by 8/4px
      • it has 2px radiuses
      • between text fields we have 8px padding
      • the input field allows line breaks and adjusts in heights according to that
      • the input field spans over the defined width of the "content" column
    • existing content of the fields is shown fully. A follow up story will extend this to do text wrapping if needed.
    • if fingerprints are uncollapsed, they are immediately in edit mode, too
    • The edit button turns into a save button
  • When the user clicks on the save button, all entries are saved in their current state. We operate under the assumption that all entity information is in valid wikidata content languages only.
    • The save icon disappears and the edit icon will be shown again
    • The page is back in reading mode
  • The edit and save buttons follow the following design specifications: (BUT if this is unexpectantly not easy or timeconsuming, please stop)
    • colors: default: #3366cc, hover: #447ff5 (not relevant for touch, but for people using mobile on desktop), active (upon clicking the button for 0,5 s): #2a4b8d
  • If other changes had been made since the users started editing, the new info is reflected when the saved object is shown, without any extra notice to the user
  • The new editing behavior does not interfere with general edit conflict detection

Mocks
Figma file containing all the design specifications / mocks



Notes

  • This story does not contain cancel mode, and does not catch errors
  • Placeholders are covered in T217000

Details

Related Gerrit Patches:
mediawiki/extensions/Wikibase : masterUpdate Termbox

Related Objects

Event Timeline

Lea_WMDE set the point value for this task to 13.

During the task breakdown we made the following assumptions:

  • switching the termbox to edit mode is for the termbox only (not enabling global edit mode)
  • we use wbeditentity for saving and will not generate custom edit summaries. This will result in generic "Changed an Item" summaries for every save.
Tarrow added a subscriber: Tarrow.Mar 20 2019, 10:21 AM

In the daily today we talked about:

wbeditentity for saving and will not generate custom edit summaries. This will result in generic "Changed an Item" summaries for every save.

and agreed for this story that was fine. We will meet in storytime to try and figure out the way forward with this in a new story or some alternative plan. We thought that the ideal solution may be an improved wbeditentities with "smart edit summaries" similar to that suggested for Lexeme in: T191885

We didn't yet decide if we could or could not ship this story to wikidata.org with the "generic" edit summary.

@Hanna_Petruschat_WMDE Hello! Matthias and I just got a little confused about one of the design specifications

the spacing between the entry fields changes according to the mocks to 48px

Does that refer to the input height or some vertical or horizontal spacing between the fields? We saw the 48px input height, but were unsure what "spacing" means here. Thanks!

NB: If this story is unexpectedly more complicated, let's discuss before everything is implemented

Pablo-WMDE updated the task description. (Show Details)Mar 27 2019, 1:23 PM

So, the mock suggests that "|" will be used for inputting multiple aliases.
Currently in the wikidata UI one can enter aliases with |s in them
Example: https://test.wikidata.org/w/index.php?title=Q214&diff=489761&oldid=2181
The special page is actually destructive here and loading the page for the entity and clicking save results in loosing data: https://test.wikidata.org/w/index.php?title=Q214&diff=489762&oldid=489761
I filed a task for that @ T219499

At the end of last year there were 56k aliases with the | character in them, so this is an issue that probably should not be ignored.
As a very minimum requirement, the term box should not allow the user to perform destructive edits as outlines in T219499, perhaps by just disallowing editing for the initial version? with a message saying why?

The alias handling of this story is just the first level. Planned final behavior is T218690: Show each alias in a separate line in edit mode, which I think should remove the problem

Tarrow added a comment.Apr 8 2019, 9:17 AM

Just to help with reviewing: as part of this task we had to make sure that people won't leak their IP addresses; this can be "product tested" as follows:

  • log into beta wikidata
  • open a page with the new term box
  • duplicate the tab
  • in one tab log out
  • in the other tab enter edit mode
  • make an edit and click save
  • check that there is now no edit in that entity's history
Tarrow updated the task description. (Show Details)Apr 8 2019, 11:32 AM
Tarrow updated the task description. (Show Details)
Tarrow updated the task description. (Show Details)Apr 8 2019, 11:36 AM
Tarrow updated the task description. (Show Details)Apr 8 2019, 11:41 AM
Tarrow updated the task description. (Show Details)Apr 8 2019, 11:44 AM
Pablo-WMDE updated the task description. (Show Details)Apr 8 2019, 1:47 PM

Change 503271 had a related patch set uploaded (by Jakob; owner: Jakob):
[mediawiki/extensions/Wikibase@master] Update Termbox

https://gerrit.wikimedia.org/r/503271

Change 503271 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Update Termbox

https://gerrit.wikimedia.org/r/503271

Pablo-WMDE added a subscriber: Pablo-WMDE.EditedApr 12 2019, 1:24 PM

Tried it on beta, looks ok given the feature set we expect to be in per the AC. Latest edits on Q11 were actually done using termbox.

Sample API call (logged in as Pagr):

@Lea_WMDE @Hanna_Petruschat_WMDE Please do the honors

Heyho,
things that I found:

  • I could not save the page, neither logged in nor logged out, neither firefox nor google. Clicking on the save button resulted in no visible response (and the data was not updated either)
  • The acceptance criteria state that line breaks are allowed, but they are not. I do remember some discussion among devs + UX that might have resulted in a ticket that states the opposite, but just in case that was not the case, I'm mentioning it here
  • The save button does not have any shadow, but it does have it in the mocks
Pablo-WMDE added a comment.EditedApr 15 2019, 3:27 PM

Hello @Lea_WMDE,

please find the following responses to the findings

I could not save the page, neither logged in nor logged out, neither firefox nor google. Clicking on the save button resulted in no visible response (and the data was not updated either)

Am I right in assuming that you used the m.wikidata.beta.wmflabs.org domain? I filed T220995 so we address the blocked cross-origin response. Please use the non-mobile domain with the useformat parameter to work around this.

The acceptance criteria state that line breaks are allowed

I'm afraid this is a phrasing problem. My interpretation always was "text is wrapped" rather than "I can input new line characters". AFAIK the way it stands is in line with UX expectations but please verify.

The save button does not have any shadow, but it does have it in the mocks

The shadow is a feature that will only come into effect when the button is "sticky" - evidence of this requirement can be found in T220381. I thought there was a dedicated ticket for the sticky button somewhere in the backlog which should duly contain this requirement but I could not find it.

Thanks for the responses, @Pablo-WMDE !

Am I right in assuming that you used the m.wikidata.beta.wmflabs.org domain? I filed T220995 so we address the blocked cross-origin response. Please use the non-mobile domain with the useformat parameter to work around this.

I can successfully make edits now :)
What does not happen, yet, is an automatic update if other people changed other parts of the item in the mean time. So if I change the de-label and somebody else changes the fr-label, I don't see the fr-label change, not even if I enter the editing mode for a second time. Only a hard refresh shows me the actual data. This is contrary to the second to last acceptance criterion, but I would be ok with leaving it like this for now (especially since I believe to remember that we put the criterion in such a way because it was easiest for you to implement).

When I create an edit conflict, right now I seem to receive an error, and I also receive an error (with error message ;) )on Desktop Wikidata. I assume that this is the right error happening in the background and should check again once we have error handling :)

The acceptance criteria state that line breaks are allowed

I'm afraid this is a phrasing problem. My interpretation always was "text is wrapped" rather than "I can input new line characters". AFAIK the way it stands is in line with UX expectations but please verify.

I will check back with @Hanna_Petruschat_WMDE , but my vague memory tells me she was against newlines, and we wanted to have text wraps anyways, so sounds great to me.

The shadow is a feature that will only come into effect when the button is "sticky" - evidence of this requirement can be found in T220381. I thought there was a dedicated ticket for the sticky button somewhere in the backlog which should duly contain this requirement but I could not find it.

Cool thanks, I was not aware of that :)

All of the above is lots of text to say: Cool stuff!
I have one remaining open question though, before moving the story to done: I wanted to check if I can edit historic revisions with the new termbox, but I did not manage to ever access a historic revision in mobile view. Is this an unrelated bug, or could you provide me with a url that I could check?

I will check back with @Hanna_Petruschat_WMDE , but my vague memory tells me she was against newlines, and we wanted to have text wraps anyways, so sounds great to me.

Yes. That's correct. So Can we consider this done?

if I change the de-label and somebody else changes the fr-label, I don't see the fr-label change, not even if I enter the editing mode for a second time. Only a hard refresh shows me the actual data

This is indeed a nice find. I share the memory that we put the AC b/c we assumed that is what the response to the save request would yield. We should evaluate if continuing like this is ok.

When I create an edit conflict, [...]. I assume that this is the right error happening in the background and should check again once we have error handling :)

We did not touch the backend merging strategy and consequently there is no risk of having broken it. You are right in assuming that the error happens but is not shown due to the lack of error handling - which goes to show the importance of tackling this => T219886

check if I can edit historic revisions with the new termbox,

That is a neat thing to validate - thanks for taking care of that. AKAIK there is no reasonable history view for mobile but what you can do is to use the desktop history view to pick a revision from the past, open it (by clicking on the date), and append &useformat=mobile to the URL. If all goes well you should see the revision as back when, but without edit button. This is actually implemented in wikibase, last touched during T214679 and one reason why the integration took the time it took back then.

Lea_WMDE closed this task as Resolved.Apr 18 2019, 8:30 PM
Lea_WMDE claimed this task.

Thanks again for the responses @Pablo-WMDE I hereby declare the editing master ticket officially resolved :)