Skip to content

Conversation

@ScuffedNewt
Copy link
Contributor

combines emotes extension & @SpeedyD 's extended mentions into an ajax based system for ease of use & backwards compatibility with updates

JAM\mallo and others added 29 commits September 28, 2022 15:09
commit 96e1524
Author: JAM\mallo <mallowmagicalus@Gmail.com>
Date:   Sat Apr 29 02:14:24 2023 -0400

    fix item entry collapsible

commit c4b2ae1
Author: JAM\mallo <mallowmagicalus@Gmail.com>
Date:   Wed Apr 19 22:19:29 2023 -0400

    add item "emotes" AND ext tracker

commit 07ee272
Author: JAM\mallo <mallowmagicalus@Gmail.com>
Date:   Fri Mar 31 19:50:48 2023 -0400

    no message

commit d1a8753
Author: JAM\mallo <mallowmagicalus@Gmail.com>
Date:   Fri Mar 31 13:16:00 2023 -0400

    no message

commit 1793229
Author: JAM\mallo <mallowmagicalus@Gmail.com>
Date:   Fri Mar 31 12:51:44 2023 -0400

    no message
…extension/emotes

# Conflicts:
#	.env.example
#	app/Helpers/Helpers.php
#	app/Http/Controllers/WorldController.php
#	config/lorekeeper/admin_sidebar.php
#	config/lorekeeper/extension_tracker.php
#	resources/views/world/_sidebar.blade.php
#	routes/lorekeeper/admin.php
@itinerare itinerare added the enhancement New feature or request label Jan 24, 2025
Copy link
Contributor

@SpeedyD SpeedyD left a comment

Choose a reason for hiding this comment

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

Looks like this is going to majorly clash with my #1097 PR.. Which, y'know, fair. This looks like a cleaner method, and allows for more.

...But I do not like the fact that it effectively removes a TON of backwards compatibility. Regenerating or simply editing older posts/pages/comments/etc. will break this way, as the parsed text is overwritten with the non-parsed text.. and with the code changed to what it is, it will not re-parse the existing code.

I cannot in good conscious approve this PR as it is.

And.. well, most minor note, I guess, is that the config/lorekeeper/extension_tracker.php file is re-added for some reason.. 🤔

Copy link
Contributor

@SpeedyD SpeedyD left a comment

Choose a reason for hiding this comment

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

Having re-reviewed this, I'm not sure if I see any problems.

I'm already thinking of how to properly apply the rest of my plans, especially considering configuration.. Disallowing users to use a certain option, for one. But that'll be a later problem.

ScuffedNewt and others added 2 commits April 4, 2025 20:35
…feature/mentions

# Conflicts:
#	public/css/lorekeeper.css
#	resources/views/character/_image_js.blade.php
#	resources/views/character/admin/_edit_description.blade.php
#	resources/views/character/admin/_edit_notes.blade.php
#	resources/views/comments/comments.blade.php
#	resources/views/comments/permalink_comment.blade.php
#	resources/views/js/_modal_wysiwyg.blade.php
#	resources/views/layouts/app.blade.php
#	resources/views/pages/credits.blade.php
Copy link
Contributor

@SpeedyD SpeedyD left a comment

Choose a reason for hiding this comment

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

And this develop merging.. leaves me with no choice but to decline my previous feedback.

I know this is probably the big chunk of the changes that mentions originally was, but in a different PR, all the tinymce initializations were moved to resources/views/js/_tinymce_wysiwyg.blade.php - this is where I'd suggest you move this to. :)

Copy link
Contributor

@SpeedyD SpeedyD left a comment

Choose a reason for hiding this comment

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

..and I realize now that I missed you already fixed it. My b, thanks for the conflict fixing commit though :)

'/\[thumb=([^\[\]&<>?"\']+)\]/', // matches [thumb=name]
];

$replacements = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh my god I'm re-reviewing this code and the sudden realization hits me here: all the extensions for images are .jpg. I do not know how I missed this prior but that is definitely not going to fly for a lot of images.

Copy link
Contributor

@SpeedyD SpeedyD left a comment

Choose a reason for hiding this comment

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

Something needs to be done about the file extension. .jpg is not the way to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request needs review Pull requests that are pending community review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants