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

Remove jQuery & Bootstrap #4111

Draft
wants to merge 17 commits into
base: 2.x
Choose a base branch
from
Draft

Remove jQuery & Bootstrap #4111

wants to merge 17 commits into from

Conversation

YUCLing
Copy link
Contributor

@YUCLing YUCLing commented Nov 13, 2024

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

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

Forum

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

Admin

  • components/AdminNav
  • components/CreateUserModal
  • components/UserListPage
Extensions

embed

  • forum
    • index

emoji

  • forum
    • addComposerAutocomplete
    • 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
      • 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.

marked jquery as deprecated
removed jquery from the following components:
- AbstractGlobalSearch
- Dropdown
- Page
- Tooltip
- Drawer
- DiscussionListItem
- DiscussionListPane
- IndexPage
- LogInModal
- Pane
- slidable
- liveHumanTimes
@luceos
Copy link
Member

luceos commented Nov 13, 2024

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: flarum/issue-archive#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.

@SychO9
Copy link
Member

SychO9 commented Nov 13, 2024

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.

@YUCLing
Copy link
Contributor Author

YUCLing commented Nov 13, 2024

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.

removed jquery from components:
- AutocompleteDropdown
- KeyboardNavigatable
removed jquery from components:
- EditUserModal
- FormModal
- SearchModal
- TextEditor
- UploadImageButton
- highlight
- anchorScroll
- GambitsAutocomplete
removed jquery from component: ConfirmDocumentUnload
fix: anchor scroll cannot accept string
@YUCLing YUCLing changed the title Remove jQuery Remove jQuery & Bootstrap Nov 15, 2024
@SychO9 SychO9 added this to the 3.0 milestone Nov 15, 2024
@SychO9 SychO9 added breaking-changes prio/low extension-breaking javascript Pull requests that update Javascript code labels Nov 15, 2024
removed jquery from components:
- ForumApplication
- AbstractPost
- Search
removed jquery from components:
- AvatarEditor
- CommentPost
- HeaderList
- LogInButton
- NotificationGrid
- PostMeta
- PostsPage
- PostStream
- ReplyPlaceholder
- SignUpModal
- WelcomeHero
- ComposerState
fix: wrong usage of select
fix: incorrect selector syntax
removed jquery from component: AffixedSidebar
removed jquery from core
@YUCLing
Copy link
Contributor Author

YUCLing commented Nov 18, 2024

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
Copy link
Contributor Author

YUCLing commented Nov 25, 2024

I'll put a pause to this PR, since Mithril v3 is right around (MithrilJS/mithril.js#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants