-
-
Notifications
You must be signed in to change notification settings - Fork 527
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Element handling from within Categories tree #16493
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 thanks for the port to 2.x
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Attempting to run the grunt build on this PR fails:
JS_Parse_Error [SyntaxError]: Invalid assignment
at JS_Parse_Error.get (eval at <anonymous> (/Users/opengeek/www/revo-2.x/_build/templates/default/node_modules/uglify-js/tools/node.js:18:1), <anonymous>:71:23)
at formatError (internal/util/inspect.js:1149:38)
at formatRaw (internal/util/inspect.js:919:14)
at formatValue (internal/util/inspect.js:774:10)
at inspect (internal/util/inspect.js:319:10)
at formatWithOptionsInternal (internal/util/inspect.js:1979:40)
at formatWithOptions (internal/util/inspect.js:1861:10)
at console.value (internal/console/constructor.js:328:14)
at console.log (internal/console/constructor.js:364:61)
at /Users/opengeek/www/revo-2.x/_build/templates/default/node_modules/grunt-contrib-uglify/tasks/uglify.js:144:17 {
filename: 'widgets/element/modx.tree.element.js',
line: 213,
col: 29,
pos: 7614
}
- When creating and Element in the root of it's type's tree: [element type] | ||
*/ | ||
|
||
[extractedData.type] = identifiers; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can a work around be put in place for this so I can get a release out? We are way overdue on a release.
Guys, I see what's happening here and on another one of the 2.x builds that's choking (#16467): We'd never updated the template build config like we did for 3.x. Unless there's opposition to doing so, let me go ahead and back port the 3.x setup to 2.x ... then we won't have these issues and won't have to handle code differently between the two branches. |
There's no opposition. Is this something you can get done soon? I'd like to get a release out ASAP. |
### What does it do? Replaces most legacy dependencies with current versions—with the exception of bourbon, neat, and fontawesome—and drops others that are no longer relevant (such as imageoptim). ### Why is it needed? Bring 2.x more in line with 3.x, mainly allow use of modern js features. ### How to test 1. Run the rebuild processes, including `npm update` within the `_build/templates/default` directory. 2. Run `grunt build`. 3. Clear your manager and browser cache, then browse around the manager with your console open to verify all works as expected and no errors are being reported. Note that grunt build will spit out some warnings, as the versions of bourbon and neat we need to stick with here (for now at least) are ancient and contain some long-deprecated code. I attempted to bring these dependencies up to date (including fontawesome) but there are many breaking changes that make it difficult to unwind and get everything working. Might try that later if there's enough "life" left in the 2.x line and it's deemed beneficial to do so. ### Related issue(s)/PR(s) Resolves issues with building after including the following PRs: #16493 and #16467. --------- Co-authored-by: Jason Coward <jason@opengeek.com>
44a843c
to
e0938bd
Compare
What does it do?
Adds a new tree method to properly extract Element data from the working tree Node. There are three patterns for a Node's id, but only one was being accounted for in the code. Additionally, cleans up code formatting in the affected methods.
Why is it needed?
Within the Categories tree, these actions are currently inoperable:
How to test
Related issue(s)/PR(s)
Resolves #16487 for MODX 2.x
Port of PR #16489