Prepare for separation of JS from PHP; header.inc.php done #3939
Conversation
|
i'll check out the commit on my server soon, just about to start house transitions when the new net is connected |
|
the only difference i can see with no vite manifest is the var jsAmpConfig header titles don't work. everything else seems okay without the manifest file. Is that cause it's taken out of the php and doesn't load it cause it's missing? not a big deal if you're following the process and doesn't look like it breaks anything else on the pr when you ignore it which is always good. |
|
Precisely, the manifest file needs to be present (from |
|
one log error while updating preferences/browsing around. haven't checked it yet but the path is public/dist/assets/main-8ZsXVU70.js so it must be missing the dist somewhere |
|
json from the build is missing the dist i think. addin that corrects the missing path |
|
The paths from the manifest are relative to dist, and we output as I'll see if I can reproduce it |
|
Fixed in #3940 |
| if (closeplaylist) { | ||
| $("#playlistdialog").dialog("close"); | ||
| } | ||
| closeplaylist = 1; | ||
| } | ||
|
|
||
| function showPlaylistDialog(e, item_type, item_ids) { | ||
| export function showPlaylistDialog(e, item_type, item_ids) { | ||
| $("#playlistdialog").dialog("close"); | ||
|
|
||
| var parent = this; |
There was a problem hiding this comment.
@mitchray tracked it back to here.
TypeError: a is undefined
could this be an issue where the dialog isn't actually there yet?
the div #playlistdialog doesn't exist and gets created by the function.
If i change this to the event (e), the dialog loads.
var parent = e;
is that wrong or incorrect somehow?
There was a problem hiding this comment.
set with bf69332 seems okay but just in case it's wrong
There was a problem hiding this comment.
Ah yeah the meaning of this changes now and no longer refers to the window object, I'll have to look out for that and update as I go.
Technically that change works as its just building up some properties for an object to instantiate the dialog, and e happens to be an object; fine for the time being but I'll touch it up when I make the next pass
This comment was marked as outdated.
This comment was marked as outdated.
| @@ -219,7 +219,7 @@ function handleShareAction(url) { | |||
| var tag_choices; | |||
| var label_choices; | |||
|
|
|||
| function showEditDialog(edit_type, edit_id, edit_form_id, edit_title, refresh_row_prefix, argument_string) { | |||
| export function showEditDialog(edit_type, edit_id, edit_form_id, edit_title, refresh_row_prefix, argument_string) { | |||
There was a problem hiding this comment.
another one i didn't see was object edit.
this one doesn't send the event so it's not going to work.
When i add this or event in the function it's missing the dialog buttons
jQuery.Deferred exception: dialog_buttons is not defined showEditDialog
The purpose of this is to lay the foundation for separating the Javascript/jQuery out of the PHP files, which will hopefully have the secondary benefit of making future PHPTAL migrations more straightforward.
There will be no changes to the JS logic apart from switching out the interspersed JS-from-PHP to be purely JS.
public/templates/header.inc.phphas been processed as a proof of concept; cut from 649 lines to 255.To outline the changes:
npm run dev), otherwise will load the compiled file (npm run build) based on a manifest (output is hashed so it can be cached)src/jswill be for global JS which is to be bundled, withpublic/lib/javascriptremaining for page specific JS files such as search.jswindownamespace so there are no changes needed for their use throughout the codebase and will behave as previouspublic/templates/js_globals.phpto consolidate all the global JS variables, grouped together and named according to their type e.g. AmpConfig, Preference, etc