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.

Sign up for GitHub

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

Jump to bottom

chore: major frontend JS cleanup #3609

Merged
merged 12 commits into from
May 7, 2023
Merged

chore: major frontend JS cleanup #3609

merged 12 commits into from
May 7, 2023

Conversation

davwheat
Copy link
Member

@davwheat davwheat commented Aug 21, 2022 •
edited
Loading

Changes proposed in this pull request:

This PR aims to clean up our JS to utilise more standardised syntax (specifically reducing our usage of empty text nodes as opposed to rendering nothing, cleaning up class name concatenation, and using JSX more).

  • Replace (condition ? < /> : '') with (!!condition && < />)
  • Replace most .component() calls with JSX
  • Clean up some string concatenation
  • Use classList instead of concatenation
  • Replace some array-based fragments with <></>
  • Consistently use className instead of class

Reviewers should focus on:

Screenshot

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?

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Backend changes: tests are green (run composer test).
  • Core developer confirmed locally this works as intended.
  • Tests have been added, or are not appropriate here.

Required changes:

  • Related documentation PR: (Remove if irrelevant)
  • Related core extension PRs: (Remove if irrelevant)

@davwheat davwheat added this to the 1.5 milestone Aug 21, 2022
@davwheat davwheat requested a review from SychO9 August 21, 2022 22:05
@davwheat davwheat self-assigned this Aug 21, 2022
@SychO9 SychO9 modified the milestones: 1.5, 1.6 Sep 6, 2022
@SychO9 SychO9 mentioned this pull request Sep 7, 2022
Merged
8 tasks
@SychO9 SychO9 modified the milestones: 1.6, 1.7 Nov 15, 2022
@davwheat davwheat requested a review from SychO9 February 23, 2023 14:01
@davwheat davwheat marked this pull request as ready for review February 23, 2023 14:01
@davwheat davwheat requested a review from a team as a code owner February 23, 2023 14:01
@SychO9
Copy link
Member

SychO9 commented Feb 28, 2023

tests failing 🤔 hard to tell but maybe formatting?

@davwheat
Copy link
Member Author

I've run yarn workspaces run format and yarn workspaces run build and both work fine... This is weird...

@SychO9
Copy link
Member

SychO9 commented Mar 1, 2023

seems to be core check-typings or build-typings or both

@SychO9 SychO9 modified the milestones: 1.7, 1.8 Mar 6, 2023
SychO9 added 2 commits May 7, 2023 17:00
@SychO9
Merge branch 'main' into dw/js-cleanup
2c4de2a 
# Conflicts:
#	extensions/mentions/js/src/forum/addComposerAutocomplete.js
#	extensions/mentions/js/src/forum/addMentionedByList.js
#	extensions/tags/js/src/forum/components/TagHero.js
#	extensions/tags/js/src/forum/components/TagsPage.js
#	framework/core/js/src/forum/components/ChangeEmailModal.tsx
#	framework/core/js/src/forum/components/ChangePasswordModal.tsx
#	framework/core/js/src/forum/components/DiscussionListItem.tsx
@SychO9
chore: format && fix typings
49fc2c5 
Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
@SychO9 SychO9 merged commit e63e161 into main May 7, 2023
@SychO9 SychO9 deleted the dw/js-cleanup branch May 7, 2023 16:40
@github-actions github-actions bot mentioned this pull request May 17, 2023
Closed
@SychO9 SychO9 added the javascript Pull requests that update Javascript code label May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SychO9 SychO9 SychO9 approved these changes

@datlechin datlechin datlechin approved these changes

Assignees

@davwheat davwheat

Labels
javascript Pull requests that update Javascript code type/cleanup
Projects
None yet
Milestone
1.8
Development

Successfully merging this pull request may close these issues.
3 participants
@davwheat @SychO9 @datlechin