Page MenuHomePhabricator

Initial Vue implementation for ZObject editing
Closed, ResolvedPublic

Description

This will probably be mostly a port of what was done on the old google/abstracttext project.

Event Timeline

From the standup meeting today, I'm going to try working on this over the next week or two. However, I had a basic question on how to proceed. I'll do it on its own branch for now, so merging shouldn't be an issue right away. The question: in google/abstracttext I implemented this as a separate "new edit" button, but I think it would be better to just replace the regular "edit" page for the ZObject namespace. Any strong opinions on this? This is going to be pretty experimental to start with, so of course we can change things drastically here!

Replacing the "edit" button sounds fine to me; I think eventually we'll want to merge the edit and view modes (so items of the view can be individually triggered into "edit" mode), but just replacing the Edit tab for now sounds great.

Yes, let's go with whatever is easier for the first iteration. I would eventually expect the editing be closer to the view (and more localized), similar to the way editing on Wikidata works, where there is no global edit mode.

This is moving along, but I'm noticing that currently there are some significant differences - in particular in language handling and I'm not sure how to proceed. The old 'abstracttext' had a list of languages that were themselves ZObjects - of type Z180 (language). This allowed having both the short code ('en') and a full label ('English', 'Anglais', etc.) in whatever the chosen current language was. I guess for now I'll just use and display the short codes and allow entry of whatever code they want rather than having a drop-down of languages, but maybe longer term we would want a different choice here?

This is moving along, but I'm noticing that currently there are some significant differences - in particular in language handling and I'm not sure how to proceed. The old 'abstracttext' had a list of languages that were themselves ZObjects - of type Z180 (language). This allowed having both the short code ('en') and a full label ('English', 'Anglais', etc.) in whatever the chosen current language was.

Yeah, this is a choice I made for now to make things simpler in the model handling, but also to be closer to how MediaWiki does things. The display values and accepted short-codes are available from LanguageNameUtils, thusly:

$langUtils = MediaWikiServices::getInstance()->getLanguageNameUtils();
$code = 'fr';

$validityCheck = $langUtils->isValidLanguageCode( $code );

$validCodes = array_keys( $langUtils->getLanguageNames() );


$codeOfLanguageToDisplayIn = 'nl';

$displayNameInOwnLanguage = $langUtils->getLanguageName( $code );

$displayNameInDutch = $langUtils->getLanguageName( $code, $codeOfLanguageToDisplayIn );

Later, my thought was to proxy the codes transparently into uneditable system ZObjects, but that's for the future.

I guess for now I'll just use and display the short codes and allow entry of whatever code they want rather than having a drop-down of languages, but maybe longer term we would want a different choice here?

Yeah, likely.

very rough version (it displays stuff but I don't think any edit functionality is working yet) in gerrit - in progress! https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikiLambda/+/627887
:

Yeah, this is a choice I made for now to make things simpler in the model handling, but also to be closer to how MediaWiki does things. The display values and accepted short-codes are available from LanguageNameUtils, thusly:

I've uploaded a patch to use LanguageNameUtils as suggested - however it doesn't seem to be translating the names into the user language as it claims, not sure what's going wrong there...

Yeah, this is a choice I made for now to make things simpler in the model handling, but also to be closer to how MediaWiki does things. The display values and accepted short-codes are available from LanguageNameUtils, thusly:

I've uploaded a patch to use LanguageNameUtils as suggested - however it doesn't seem to be translating the names into the user language as it claims, not sure what's going wrong there...

It seems to work for me?

English
Screenshot 2020-09-17 at 16.41.18.png (668×1 px, 399 KB)
Arabic
Screenshot 2020-09-17 at 16.44.00.png (884×1 px, 576 KB)

I'm pretty sure no language other than English has a full set at any time, but most of them are there for most of the big ones.

Not sure what's going on with the language stuff, but it sounds like it might be a local problem on my end. I'll probably reload everything at some point and see if it goes away.

I uploaded some new changes to take account of the new features of persistent ZObjects, and repurposed the language-label components as more generic MultiLingualString support (still in progress on implementing that).

And just added another patch to fix the types and labels list (temporarily - these should really come from the data itself somehow).

2 more patches to fix up ES5 vs ES6 issues (mostly const/let -> var) in the javascript, and also to address npm test and SonarQube complaints.

What do I have to do to address the "coverage" complaints though?

2 more patches to fix up ES5 vs ES6 issues (mostly const/let -> var) in the javascript, and also to address npm test and SonarQube complaints.

What do I have to do to address the "coverage" complaints though?

I can fiddle to fix a few of the issues if you want? The Sonar cloud ones can generally be ignored (they're automated suggestions, sometimes useful, sometimes wrong, and don't block merge).

Aha, the main issue is that I forgot to enable Vue linting in the repo yet; whoops. There's a lot of issues shouted about, sadly.

I've pushed a quick fiddle, mostly auto-fixed by eslint, and disabled a few of the noiser warnings that are not urgent.

Hmm, thanks for fixing those things. However, now I'm not sure how to submit my further changes - I get the following message after doing a merge with your changes and then trying 'git review -R':

"You are about to submit multiple commits. This is expected if you are
submitting a commit that is dependent on one or more in-review
commits, or if you are submitting multiple self-contained but
dependent changes. Otherwise you should consider squashing your
changes into one commit before submitting (for indivisible changes) or
submitting from separate branches (for independent changes).

The outstanding commits are:

a9a3c64 (HEAD -> editing-with-vue) [WIP] Initial Vue implementation for ZObject editing
363d1ce Merge commit 'refs/changes/87/627887/7' of ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/WikiLambda into editing-with-vue
0e5da29 [WIP] Initial Vue implementation for ZObject editing
1f44655 [WIP] Initial Vue implementation for ZObject editing
1d24d59 [WIP] Initial Vue implementation for ZObject editing
22d0726 Merge "Provide initial bootstrapping ZObjects on installation"
c145299 Localisation updates from https://translatewiki.net.
2d68f0b Provide initial bootstrapping ZObjects on installation
ec7b61c Localisation updates from https://translatewiki.net.
c584d20 Localisation updates from https://translatewiki.net.
"
Do I need to squash these down to one?

Sorry, I rebased it; if you git fetch it should understand that origin/master has changed.

Alternatively, re-pull from gerrit?

Re-pull from gerrit worked (I think). I've uploaded patchset 9 - basically I've abandoned the special handling for labels as too much has changed there from the old implementation, and it all sort of works (more crudely) with the OtherKeys implementation anyway. No language drop-down any more though... I would have had to completely rewrite those two components.
However, I'm thinking the longer term solution here is to write special components like those for each ZObject type, which will allow special things like the language handling. I sort of have special handling now for lists (Z10) and strings (Z6), though only because those are built in to the representation.

And patchset 10 - this was using an API to fetch the labels of keys in the old implementation; obviously that's not available (yet) here, and we might prefer a different solution anyway. So I stripped that out.

Re-pull from gerrit worked (I think).

Excellent.

I've uploaded patchset 9 - basically I've abandoned the special handling for labels as too much has changed there from the old implementation, and it all sort of works (more crudely) with the OtherKeys implementation anyway. No language drop-down any more though... I would have had to completely rewrite those two components.

Ack.

However, I'm thinking the longer term solution here is to write special components like those for each ZObject type, which will allow special things like the language handling. I sort of have special handling now for lists (Z10) and strings (Z6), though only because those are built in to the representation.

Agreed.

Patchset 11: call makeEmptyContent if ZObject is new

Change 627887 had a related patch set uploaded (by Jforrester; owner: ArthurPSmith):
[mediawiki/extensions/WikiLambda@master] Initial Vue implementation for ZObject editing

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

Patchset 15 fixes the issue with creating new ZObjects! However, it also unintentionally altered the package-lock.json file; I'm not quite sure what happened to change that...

and another patchset to fix eslint complaints...

Latest patchset (19) removes the Object.entries and destructuring bit; really this wasn't needed and if we're sticking with ES5 then it should go.

@santhosh there are a few places where English words are used that should be i18n messages - do you know how this should work with Vue in Mediawiki? Is there a good example out there now?

@santhosh there are a few places where English words are used that should be i18n messages - do you know how this should work with Vue in Mediawiki? Is there a good example out there now?

I know how, worry not. :-)

Change 627887 merged by jenkins-bot:
[mediawiki/extensions/WikiLambda@master] Initial Vue implementation for ZObject editing

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

Change 629419 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/WikiLambda@master] Vue: Pull out UI copy for internationalisation

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

Change 629419 merged by jenkins-bot:
[mediawiki/extensions/WikiLambda@master] Vue: Pull out UI copy for internationalisation

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