framework icon indicating copy to clipboard operation
framework copied to clipboard

Remove jQuery & Bootstrap

Open YUCLing opened this issue 1 year ago • 5 comments

This is still an ongoing work and brings huge breaking changes. Quality check and feedback are required before final merge.

Completes https://github.com/orgs/flarum/projects/22?pane=issue&itemId=5300309

Changes proposed in this pull request: This PR removes jQuery and Bootstrap completely from Flarum, which can greatly reduce the frontend size.

Reviewers should focus on:

  • [ ] Are all jQuery usages removed?
  • [ ] Are all Bootstrap usages removed?
  • [ ] Do vanilla implementations match the original jQuery/Bootstrap behaviors?
    • nullable element variable and zero-length jQuery objects
    • select multiple elements or single element
    • jQuery css returns computed style value

Necessity

  • [ ] Has the problem that is being solved here been clearly explained?
  • [ ] If applicable, have various options for solving this problem been considered?
  • [ ] For core PRs, does this need to be in core, or could it be in an extension?
  • [ ] Are we willing to maintain this for years / potentially forever?

Components using jQuery

Core

Common

  • [x] components/AutocompleteDropdown
  • [x] components/ConfirmDocumentUnload
  • [x] components/EditUserModal
  • [x] components/FormModal
  • [x] components/SearchModal
  • [x] components/TextEditor
  • [x] components/UploadImageButton
  • [x] helpers/highlight
  • [x] utils/anchorScroll
  • [x] utils/GambitsAutocomplete
  • [x] utils/KeyboardNavigatable

Forum

  • [x] ForumApplication
  • [x] components/AbstractPost
  • [x] components/AffixedSidebar
  • [x] components/AvatarEditor
  • [x] components/CommentPost
  • [x] components/Composer
  • [x] components/HeaderList
  • [x] components/LogInButton
  • [x] components/NotificationGrid
  • [x] components/PostMeta
  • [x] components/PostsPage
  • [x] components/PostStream
  • [x] components/PostStreamScrubber
  • [x] components/ReplyPlaceholder
  • [x] components/Search
  • [x] components/SignUpModal
  • [x] components/WelcomeHero
  • [x] states/ComposerState

Admin

  • [x] components/AdminNav
  • [x] components/CreateUserModal
  • [x] components/UserListPage
Extensions

embed

  • forum
    • [x] index

emoji

  • forum
    • [x] addComposerAutocomplete
    • [x] fragments/AutocompleteDropdown

mentions

  • forum
    • [ ] index
    • [ ] addComposerAutocomplete
    • [ ] addMentionedByList
    • [ ] addPostMentionPreviews
    • [ ] addPostQuoteButton
    • [ ] fragments/AutocompleteDropdown
    • [ ] fragments/PostQuoteButton
    • [ ] utils/selectedText

messages

  • forum
    • [ ] components/MessagesPage
    • [ ] components/MessageStream

nicknames

  • forum
    • [ ] index

subscriptions

  • forum
    • [ ] components/SubscriptionMenu

suspend

  • forum
    • [ ] components/SuspendUserModal

tags

  • admin
    • [ ] components/TagsPage
  • common
    • [ ] components/TagSelectionModal
  • forum
    • [ ] addTagComposer

Additional Work

  • [ ] Optimize the repeated code block in Search and AutocompleteDropdown
  • [ ] Cleanup affix left overs if possible
  • Utility functions
    • [ ] Get element offset to document
    • [ ] Get element dimension with margin
      • [x] height
      • [ ] width?

Changed behaviors

Some features are not available in vanilla JS and is not worth implementing them. They are listed below:

  • jQuery animated scroll

Confirmed

  • [ ] Frontend changes: tested on a local Flarum installation.
  • [ ] Core developer confirmed locally this works as intended.

YUCLing avatar Nov 13 '24 17:11 YUCLing

Hello @YUCLing,

Thank you so much for opening a Draft PR for this. Looking at your description and title, I think we originally wanted to not just drop jQuery, but also Bootstrap, see this archived issue as an example: https://github.com/flarum/issue-archive/issues/179.

Right now I don't exactly know if we can and should pursue that in the scope of this overhaul. But generally speaking, if you touch something that breaks all extensions, it might as well touch more.

@flarum/core opinions appreciated.

luceos avatar Nov 13 '24 18:11 luceos

Like I mentioned in the discuss thread. Really appreciate the effort 🙏

The changes will very very likely still be relevant for another major version, just not at this time right for 2.0 as it's a little late, but also because it is already difficult as it is upgrading extensions for 2.0 with all the breaking changes already introduced.

SychO9 avatar Nov 13 '24 19:11 SychO9

but also Bootstrap

That can be done too. But I would like to focus on jQuery first, then Bootstrap. Just split them into small tasks.

The changes will very very likely still be relevant for another major version

It's not a big problem. This PR doesn't have to be merged now.

~IMO, It can still be merged into 2.0. We can just mark the jQuery API as deprecated in 2.0 to discourage the usage and remove the usages in the core itself, leaving jQuery in place so it's not a breaking change. Doing so can make the transition in next major version easier.~

Nevermind, if some extensions extend/override the functions that returns jQuery, that will be a problem.

YUCLing avatar Nov 13 '24 23:11 YUCLing

Hi @SychO9 @luceos

I'm planning to introduce a new library (https://github.com/floating-ui/floating-ui) to take care of things like Tooltip and Dropdown's positioning, what's your opinion?

YUCLing avatar Nov 18 '24 15:11 YUCLing

I'll put a pause to this PR, since Mithril v3 is right around (https://github.com/MithrilJS/mithril.js/pull/2982). This PR is scheduled for 3.x, which is still far away.

I'm gonna decide next steps after Mithril v3 is settled. It might turn to a big overhaul to frontend instead of just removing jQuery & Bootstrap.

YUCLing avatar Nov 25 '24 09:11 YUCLing