Drop dependency on shared-resources #309

wolfgangmm · 2021-06-08T14:27:46Z

This PR drops all dependencies on shared-resources. It also modernizes the build to facilitate future updates and replaces some (though not all) of the old components, which were depending on jquery/jquery-ui.

TODO:

replace the table grid used in the file browser. The old one shows random bugs (screen remains empty), is no longer maintained and has dependencies on outdated jquery and jquery-ui versions

…y dependency for gid

joewiz · 2021-06-08T21:51:54Z

package.json

@@ -1,6 +1,6 @@
 {
 	"name": "exide",
-	"version": "2.4.13",
+	"version": "2.5.0",


I think due to the removal of the app generator this might merit a 3.0.

I tend to agree

joewiz · 2021-06-08T22:00:54Z

This is great, Wolfgang!

I've been testing, and other than the error reported by npm run cypress, the app builds and functions great. The new File > Manage window is great - I love being able to quickly filter the resources:

... and the advanced filters:

The only minor issue is that the right-most column, "Last Modified Date", is too narrow to display the date times, and AFAIK the column width can't be adjusted:

I think it's a great idea to use the repo to view/maintain the app's documentation.

I'll keep using this and will report anything I see.

joewiz · 2021-06-09T16:05:15Z

@wolfgangmm The default width of the last modified column is wonderful, and the resizable columns are working great!

I noticed one slight change: in the old file manager it was possible to rename a resource by clicking in the resource name field. I'm not able to trigger the rename mode. It was always a little fiddly, though; it was easy to accidentally trigger unintended behavior—either entering rename mode or opening the resource. Perhaps a dedicated Rename button with a modal dialog for renaming resources would be better? In any case, I just wanted to note this issue.

wolfgangmm · 2021-06-09T16:08:58Z

@joewiz Yes, I dropped the rename feature because with the new component it is too easy to accidentally activate it. A toolbar button to click for activating the cell editor could indeed solve this.

package.json

line-o

This looks like a massive improvement.

package.json

line-o · 2021-06-10T11:33:55Z

Well, when I try to build the project I get following error

➜ npm run build

> exide@2.5.0 build
> node scripts/build.js prepare && npm run xqlint && node scripts/build.js


> exide@2.5.0 xqlint
> cd support/xqlint && node r.js -o build.js && cd ../..

/Users/jll/customers/ib/eXide/support/xqlint/r.js:2267
        if (path.existsSync(url)) {
                 ^

TypeError: path.existsSync is not a function
    at requirejsVars.nodeLoad.req.load (/Users/jll/customers/ib/eXide/support/xqlint/r.js:2267:18)
    at resume (/Users/jll/customers/ib/eXide/support/xqlint/r.js:1282:65)
    at Object.require (/Users/jll/customers/ib/eXide/support/xqlint/r.js:1481:25)
    at requirejs (/Users/jll/customers/ib/eXide/support/xqlint/r.js:1684:24)
    at /Users/jll/customers/ib/eXide/support/xqlint/r.js:12506:1
    at Object.<anonymous> (/Users/jll/customers/ib/eXide/support/xqlint/r.js:12556:2)
    at Module._compile (internal/modules/cjs/loader.js:1068:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1097:10)
    at Module.load (internal/modules/cjs/loader.js:933:32)
    at Function.Module._load (internal/modules/cjs/loader.js:774:14)

Any pointers or ideas on the above?

wolfgangmm · 2021-06-10T11:40:56Z

@line-o the submodule support/xqlint it should be at 36c142b12a642a5693c744afe2a0ac69c184fc9b. Maybe you still have an earlier version checked out?

line-o · 2021-06-10T11:49:08Z

git submodule ... that's it

line-o · 2021-06-10T12:18:15Z

Great work ... now that I was able to follow the readme and build it ;)

I found one issue, though, with the newly added filter capability in the file manager:

The second filter - that can be AND or OR - does not seem to taken into account. It seems that it may even reset the filter completely.
I think looking at the screenshots is easier to understand:

line-o · 2021-06-10T12:19:03Z

I would suggest to remove the second filter for now (hoping that it is easy to accomplish)

line-o · 2021-06-10T16:36:40Z

The rename feature was a nuisance often times.

line-o · 2021-06-10T19:13:45Z

src/commands.js

-        		    		eXide.find.Modules.select(doc.syntax);
-        		    	}
-        		    });
+                    // commands.addCommand({


I would remove dead code

line-o · 2021-06-10T19:14:51Z

src/eXide.js

-            eXide.find.Modules.addEventListener("import", null, function (module) {
-                editor.exec("importModule", module.prefix, module.uri, module.at);
-            });
+            // eXide.find.Modules.addEventListener("open", null, function (module) {


same here - or do we need to fix it?

We may want to re-implement the feature sometimes, so I'll rather leave it commented out.

Well, then it is still in version control. It may never come back and would then stay there forever.
Would you be against me removing it?

…d conflicts

wolfgangmm · 2021-06-14T11:37:00Z

The rename feature is back. You now need to click the 'rename' button in the toolbar first to switch into edit mode. This avoids any conflicts with mouse and keyboard navigation.

line-o · 2021-06-14T15:16:53Z

OK, everything is green. The last thing that keeps me from merging it is the discussion about dead code removal.
We could postpone this, though.
So if $someone merges this I am OK with it.

line-o · 2021-06-14T15:17:44Z

Oh and the version discussion ... I am in team 3.0.0

joewiz · 2021-06-14T15:56:30Z

@wolfgangmm Thank you! The rename function works well. I just noticed one issue with it: if the user selects a cell from a field other than Name and then selects the Rename button, the contents of that cell become available for editing—instead of the contents of the Name field. Users might misunderstand that they can use this tool to change the permissions, user, group, or date of resources. Here's a screenshot showing the date field being editable:

If a user changes the value of one of these non-Name fields and hits return to submit the rename request, a "Rename error" dialog is validly shown: "Cannot move resource to itself '/db/test2.xlsx'. XMLDB exception caught: Cannot move resource to itself '/db/test2.xlsx'." The accompanying request URL: http://localhost:8080/exist/apps/eXide/modules/collections.xql?target=test2.xlsx&rename=test2.xlsx&root=%2Fdb%2F. Screenshot:

Ideally, when the Rename action is triggered, only the Name field would be made editable, regardless of which cell is selected in the row.

Do you have a sense of whether this would be possible in this PR, or would you prefer to defer this to a future PR? I could open an issue, and we could list it as a known issue in the release.

@line-o Thanks for your fixes allowing CI checks to work with this PR!

line-o · 2021-06-14T17:25:02Z

@wolfgangmm thanks for the hard work. This looks very well rounded now. Will merge after tests have run.

joewiz

I've tested the merged PR and can confirm that the Rename button only has effect if you've selected a resource's Name field. Thank you for everything in this PR, @wolfgangmm!

wolfgangmm added 14 commits June 4, 2021 10:45

Add required ace version as submodule

a29f91c

Use bundled ace version

c158704

Move documentation to separate .md

90195a3

Get rid of remaining deps on shared-resources

e6abd81

Correct links to documentation

3c1dbff

Remove app generation feature

2575f1a

Remove dependency on shared resources

df80913

Switch build from ant to esbuild

fe991dd

Use fixed version of xqlint

105c00d

Fix build

fb6e363

Remove closure compiler

e495723

Fix another hidden dependency on jquery

36eef83

Replace outdated and unmaintained slickgrid with ag-grid; drops jquer…

8d7b14c

…y dependency for gid

Update version to 2.5.0

46129ad

joewiz reviewed Jun 8, 2021

View reviewed changes

wolfgangmm added 2 commits June 9, 2021 14:14

File manager: make columns resizable

ac6e90b

File browser: fix for opening currently selected document

c4821f0

joewiz mentioned this pull request Jun 9, 2021

[Master issue] Prepare eXist 5.3.0 stable release eXist-db/exist#3914

Closed

38 tasks

joewiz reviewed Jun 9, 2021

View reviewed changes

package.json Show resolved Hide resolved

line-o approved these changes Jun 10, 2021

View reviewed changes

package.json Show resolved Hide resolved

line-o reviewed Jun 10, 2021

View reviewed changes

src/commands.js

eXide.find.Modules.select(doc.syntax);

}

});

// commands.addCommand({

Copy link

Member

line-o Jun 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove dead code

line-o reviewed Jun 10, 2021

View reviewed changes

wolfgangmm added 3 commits June 14, 2021 12:46

Re-enable rename feature

5b6201f

Resource browser: restrict filter options to single 'contains' filter

334fa43

Resource browser: enable editing only after activated by user to avoi…

746ac51

…d conflicts

wolfgangmm changed the title ~~WIP: Drop dependency on shared-resources~~ Drop dependency on shared-resources Jun 14, 2021

line-o added 3 commits June 14, 2021 16:34

fix travis build configuration

0d33e1a

fix order of commands in travis conf

72b88f2

switch to nodejs build in travis conf

5a24c65

joewiz and others added 4 commits June 14, 2021 12:56

[ignore] Add cypress artifacts to gitignore

05b3fe0

[ignore] Add cypress artifacts to gitignore

c88733b

Resource browser: allow cell editing in filename only

7337722

Increment version to 3.0.0

41a6dfe

line-o merged commit a40f643 into eXist-db:develop Jun 14, 2021

joewiz reviewed Jun 14, 2021

View reviewed changes

joewiz mentioned this pull request Jul 8, 2021

Adopt new templating lib #292

Closed

joewiz mentioned this pull request Jul 19, 2021

[BUG] Restore dbutil module, formerly supplied by shared-resources #333

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drop dependency on shared-resources #309

Drop dependency on shared-resources #309

wolfgangmm commented Jun 8, 2021 •

edited

Loading

joewiz Jun 8, 2021

line-o Jun 10, 2021

joewiz commented Jun 8, 2021

joewiz commented Jun 9, 2021 •

edited

Loading

wolfgangmm commented Jun 9, 2021

line-o left a comment

line-o commented Jun 10, 2021

wolfgangmm commented Jun 10, 2021

line-o commented Jun 10, 2021

line-o commented Jun 10, 2021

line-o commented Jun 10, 2021

line-o commented Jun 10, 2021

line-o Jun 10, 2021

line-o Jun 10, 2021

wolfgangmm Jun 14, 2021

line-o Jun 14, 2021

wolfgangmm commented Jun 14, 2021

line-o commented Jun 14, 2021

line-o commented Jun 14, 2021

joewiz commented Jun 14, 2021

line-o commented Jun 14, 2021

joewiz left a comment

Drop dependency on shared-resources #309

Drop dependency on shared-resources #309

Conversation

wolfgangmm commented Jun 8, 2021 • edited Loading

joewiz Jun 8, 2021

Choose a reason for hiding this comment

line-o Jun 10, 2021

Choose a reason for hiding this comment

joewiz commented Jun 8, 2021

joewiz commented Jun 9, 2021 • edited Loading

wolfgangmm commented Jun 9, 2021

line-o left a comment

Choose a reason for hiding this comment

line-o commented Jun 10, 2021

wolfgangmm commented Jun 10, 2021

line-o commented Jun 10, 2021

line-o commented Jun 10, 2021

line-o commented Jun 10, 2021

line-o commented Jun 10, 2021

line-o Jun 10, 2021

Choose a reason for hiding this comment

line-o Jun 10, 2021

Choose a reason for hiding this comment

wolfgangmm Jun 14, 2021

Choose a reason for hiding this comment

line-o Jun 14, 2021

Choose a reason for hiding this comment

wolfgangmm commented Jun 14, 2021

line-o commented Jun 14, 2021

line-o commented Jun 14, 2021

joewiz commented Jun 14, 2021

line-o commented Jun 14, 2021

joewiz left a comment

Choose a reason for hiding this comment

wolfgangmm commented Jun 8, 2021 •

edited

Loading

joewiz commented Jun 9, 2021 •

edited

Loading