-
Notifications
You must be signed in to change notification settings - Fork 5
Fix/webpack #233
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
Closed
Closed
Fix/webpack #233
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
20da616
move all deps to dev, and dont minify ulabel.js
TrevorBurgoyne 0e3d8db
unit tests for both builds
TrevorBurgoyne 83cffc3
e2e tests for both builds
TrevorBurgoyne 688378c
fix npm audit issues
TrevorBurgoyne 77505c4
default to minified
TrevorBurgoyne f554c99
update changelog
TrevorBurgoyne 193efe6
Apply changes from review
TrevorBurgoyne 574c519
Merge branch 'feature/image-filters' into fix/webpack
TrevorBurgoyne 916d181
rebuild after merge
TrevorBurgoyne File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,35 +1,48 @@ | ||
| ## Tasks | ||
| - [x] Read the description in [#128](https://github.com/SenteraLLC/ulabel/issues/128) | ||
| - [x] Update this checklist with steps to take in order to implement the requested functionality | ||
| - [x] Read the description in [#164](https://github.com/SenteraLLC/ulabel/issues/164) | ||
| - [x] Write a clear summary of the requested change | ||
| - [x] Propose some options of how to proceed. Wait for user input to decide which to try first. | ||
|
|
||
| ### Implementation Steps | ||
| - [x] Design and implement UI for image filters | ||
| - [x] Create a collapsible/popup menu component for filter controls | ||
| - [x] Add sliders for each CSS filter property: | ||
| - [x] Brightness (0-200%, default 100%) | ||
| - [x] Contrast (0-200%, default 100%) | ||
| - [x] Hue-rotate (0-360deg, default 0) | ||
| - [x] Invert (0-100%, default 0%) | ||
| - [x] Saturate (0-200%, default 100%) | ||
| - [x] Add reset button to restore default values | ||
|
|
||
| - [x] Implement filter state management | ||
| - [x] Add filter state to configuration/state management | ||
| - [x] Make default filter values configurable | ||
| - [x] Add getter/setter methods for filter values | ||
| ### Decision: Proceeding with Option 3 (Webpack modernization + dependency cleanup) | ||
|
|
||
| - [x] Apply CSS filters to canvas/image | ||
| - [x] Apply CSS filter property to the image canvas element only (not whole screen) | ||
| - [x] Update filter values dynamically as sliders change | ||
| - [x] Ensure filters don't affect UI elements (text, buttons, etc.) | ||
| #### Step 1: Move dependencies to devDependencies | ||
| - [x] Move `@turf/turf` from dependencies to devDependencies | ||
| - [x] Move `jquery` from dependencies to devDependencies | ||
| - [x] Move `polygon-clipping` from dependencies to devDependencies | ||
| - [x] Move `uuidv4` from dependencies to devDependencies | ||
| - [x] Test: Run `npm install` and verify it works | ||
| - [x] Test: Run `npm run build` and verify output is identical (both files 1039.6 KB) | ||
|
|
||
| - [x] Testing | ||
| - [x] Test all filter controls work correctly | ||
| - [x] Verify filters only apply to image, not UI | ||
| - [x] Test filter combinations | ||
| - [x] Run linting: `npm run lint` | ||
| - [x] Run build: `npm run build` | ||
|
|
||
| - [x] Documentation | ||
| - [x] Update API documentation with new filter methods | ||
| - [x] Tested on demo page (multi-class demo works great!) | ||
| #### Step 2: Enable and modernize webpack minification ✅ COMPLETE | ||
| - [x] Remove commented-out UglifyJsPlugin code (deprecated) | ||
| - [x] Enable webpack 5's built-in TerserPlugin for minification | ||
| - [x] Configure it to only minify `ulabel.min.js`, not `ulabel.js` | ||
| - [x] Test: Run `npm run build` and verify both files are created | ||
| - [x] Test: Verify `ulabel.js` is NOT minified (readable) - 2.33 MB with webpack runtime and formatted code | ||
| - [x] Test: Verify `ulabel.min.js` IS minified (smaller size) - 1.02 MB minified | ||
| - [x] Test: Run `npm run lint` - No errors | ||
| - [x] Note: File size difference (2.33 MB vs 1.02 MB) is expected - readable version includes webpack runtime overhead | ||
|
|
||
| #### Step 3: Verify and document ✅ COMPLETE | ||
| - [x] Compare file sizes before/after | ||
| - Before: Both files 1039 KB (both minified, minification was disabled) | ||
| - After: ulabel.js 2.33 MB (readable), ulabel.min.js 1.02 MB (minified) | ||
| - Result: Minification now working correctly, file size increase for non-min version is expected | ||
| - [x] Update CHANGELOG.md with changes | ||
| - [x] Document any findings or recommendations | ||
|
|
||
| #### Step 4: Security and dependency updates ✅ COMPLETE | ||
| - [x] Run `npm audit` to identify vulnerabilities | ||
| - [x] Fix 12 vulnerabilities using `npm audit fix` | ||
| - [x] Apply breaking changes for remaining issues with `npm audit fix --force` | ||
| - [x] Update `typescript-eslint` packages to be compatible with ESLint 9.37.0 | ||
| - [x] Fix linting issues in `tests/e2e/fixtures.js` | ||
| - [x] Verify all tests still pass (28 unit tests + 36 e2e tests) | ||
| - [x] Final audit: 0 vulnerabilities | ||
|
|
||
| #### Step 5: Configure package exports for minified by default ✅ COMPLETE | ||
| - [x] Update `main` and `module` fields to point to `dist/ulabel.min.js` | ||
| - [x] Add `exports` field with options: `.` (minified), `./min` (minified), `./debug` (unminified) | ||
| - [x] Update `unpkg` field to serve minified version by default | ||
| - [x] Update README.md with usage examples for both minified and unminified versions | ||
| - [x] Document clear import patterns for users |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Large diffs are not rendered by default.
Oops, something went wrong.
Large diffs are not rendered by default.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does have the downside of adding a massive diff to every PR. Not a huge deal, although I would lean on the side of having all of the builds done as a GH Action upon merging PRs into main (which would also automate away one step of the current PR checklist).