Skip to content
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

feat: mentions #1205

Open
wants to merge 36 commits into
base: develop
Choose a base branch
from
Open

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 <[email protected]>
Date:   Sat Apr 29 02:14:24 2023 -0400

    fix item entry collapsible

commit c4b2ae1
Author: JAM\mallo <[email protected]>
Date:   Wed Apr 19 22:19:29 2023 -0400

    add item "emotes" AND ext tracker

commit 07ee272
Author: JAM\mallo <[email protected]>
Date:   Fri Mar 31 19:50:48 2023 -0400

    no message

commit d1a8753
Author: JAM\mallo <[email protected]>
Date:   Fri Mar 31 13:16:00 2023 -0400

    no message

commit 1793229
Author: JAM\mallo <[email protected]>
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
@itinerare itinerare added the needs review Pull requests that are pending community review 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.

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