- Notifications
You must be signed in to change notification settings - Fork693
Add a "Fast Mod" Array Editor#1668
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Draft
bassmanitram wants to merge58 commits intojson-editor:masterChoose a base branch
base:master
Could not load branches
Branch not found:{{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline, and old review comments may become outdated.
Draft
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
Main enhancements:* Cleanup and alignment of Table and Array editor code* New option `array_copy_in_place` determines whether a copied element is placed after the element it is copied from, or placed at the end of the array. For `array` the default value is `false` corresponding to the existing behaviour. For `table` the default value is `true` corresponding to the existing behaviour.
Rather than the potentially expensive validation of a valueagainst multiple schemata, how about we provide the path toa value field that tells you the type and an array of values inthe order of declaration!
This change removes the data-index item, and instead discovers the index of the currently active tab - this allowsthe tabs to be moved without refreshing the value - makingmove, copy-in-place and drop WAY faster (not here but ina subclass)Also a fix to the hard-usage of the index in the editor path.As before, instead of using the row index we use the 'editorId' set by the class/subclass. (in array.js that IS theindex, but in a subclass that, say, forgoes the use of the cache and, instead, uses editor paths NOT tied to the arrayitem order, the implementation of move, delete, copy, add, and drop can all be vastly improved, and in a way that does not degrade with the increasing size of the array).
The tiny increase in resources is justified in order to allowsubclasses to use a different strategy for row order management(See FastModArrayEditor)
All button event listeners now call an instance method for themain implementation, allowing subclasses to implement differentstrategies (e.g. see FastModArray). Those event listener MAYchoose to return a value that indicates to the event listener thatthe remaining functionality of the listener should be executed.Mostly that's specifically returning `true` - which is backwardcompatible to the original array semantics. For one case, where anobject is expected, _not_ rturning that object has the same effect.
(and this doesn't fix it)
If run standalone - there appear to be a lot oftiming issues if run in the full suite
It's not worth the trouble they cause!
The test case no longer uses unnecessary popups
(In particular, being able to remove button labels in subeditors)
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Labels
None yet
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading.Please reload this page.
NOTE: Depends on PR#1667
This is essentially the standard Array editor with a tweak to the "Editor ID" that then allows it to NOT fully reset the value when simply modifying the order of the items in the array, making such reordering perform better, and to not experience degraded performance as the array grows in size.
The key "innovation" is to not tie the editor ID to its position in the array. Technically this would be possible with most of the functionality of the Array editor, even with the initial reliance on array index for the editor ID. However it is such a major change in semantics that I felt it better to implement the innovations via a subclass.
The major semantics change is, of course, that you cannot "construct" the path of an item editor based upon its position in the array. Instead you have to get the array editor, then index into the rows array, then obtain the path from the target row (or the
editorIdproperty of that row and append it to the path of the array editor).Also, technically the cache SHOULD still work with a few tweaks - but for now I completely disable the cache.
As part of developing this subclass, the array editor itself was refactored quite a bit so that it didn't rely upon array position for so many of its operations where it didn't need to, even though it still does guarantee the tie between item index and item editor path, and to allow functionality to be overridden by subclasses. The cache, in particular, has been "objectified".
The bottom line, if you have a big array and you don't need to find item editors based upon their position in the array, then this subclass may afford you better performance when modifying the array, especially if such modifications are predominantly order changes.