Skip to content

TASK: Modernize code and bump dependencies #91

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

Merged
merged 13 commits into from
Jan 31, 2023
Merged

Conversation

Sebobo
Copy link
Member

@Sebobo Sebobo commented Jan 23, 2023

What has changed:

  • Use latest React, TypeScript, Parcel, etc...
  • Target modern browsers
  • Reduce bundle size from 450KB to 350KB
  • Require at least Neos 7.3 and PHP 7.3
  • Use yarn workspaces
  • Use "Lite" stylesheet from Neos
  • Adjust resource inclusion setting to allow additional resources via settings
  • Update linting and resolve issues
  • Apply PHP code cleanup hints to controller

Only support Neos 7.3 and 8.x+ and require at least PHP 7.4
By defining the resources as assoziative array instead of an indexed array its much easier to add custom styles or scripts to the module.
This prevents collisions between the hard CSS reset in the default stylesheet
@Sebobo
Copy link
Member Author

Sebobo commented Jan 23, 2023

We should define a new minor version when this is released as it's not breaking but has bumped dependencies.

@Sebobo Sebobo requested a review from jonnitto January 23, 2023 10:10
@Sebobo
Copy link
Member Author

Sebobo commented Jan 23, 2023

The largest part of the bundle are react-com and the react-datepicker. I don't know right now how to reduce this further.

Copy link
Member

@jonnitto jonnitto left a comment

Choose a reason for hiding this comment

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

Tested it and it works

@Sebobo Sebobo requested a review from ahaeslich January 23, 2023 10:52
Copy link
Member

@ahaeslich ahaeslich left a comment

Choose a reason for hiding this comment

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

The linter check fails in _add-redirect-form.css on line 59. The import of @import "../vendor/datepicker"; seems a bit odd.

@Sebobo can we delete this or do we have to include something else?

@crydotsnake
Copy link

crydotsnake commented Jan 25, 2023

Sorry for my missing review so far. Will take a look at the PR this evening!

Copy link

@crydotsnake crydotsnake left a comment

Choose a reason for hiding this comment

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

Tested, and it works!

@Sebobo Sebobo force-pushed the task/modernize-code branch from 924fc49 to b2aa6f2 Compare January 31, 2023 14:41
@Sebobo
Copy link
Member Author

Sebobo commented Jan 31, 2023

The linter check fails in _add-redirect-form.css on line 59. The import of @import "../vendor/datepicker"; seems a bit odd.

@Sebobo can we delete this or do we have to include something else?

There was a reason due to specificity. So I disabled the style lint rule.

@Sebobo Sebobo merged commit f318460 into master Jan 31, 2023
@Sebobo Sebobo deleted the task/modernize-code branch January 31, 2023 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants