Remove fix that cloned path segments and add tests to show it works #69
Conversation
This file contains 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
Closed
|
I think that was one of my workarounds, oddly I can't see the bug in github issues (maybe it was referenced from Google group, but I could have sworn there was an issue on git). The bug was where after about 3 points created and adjusted in the pathtool, there would be an error in the console, and the path would be deleted. The cloning was a way to get around a subtle issue under the hood that I found hard to understand iirc, so was never quite comfortable with my fix, so glad its all working correctly now. |
|
Thanks for confirming ibrierley. That case seems to work with the patch but I think there's a good chance a real bug is hiding down there somewhere. It's also possible that it was a browser bug that's been fixed. I think the best approach will be to land this patch, and then separately look into any bugs that crop up as a result. |
|
Is there a version running with the commit out of interest, to take a peek ? |
|
@ibrierley, sure thing: |
|
Thanks, can confirm that the browsers I had been testing in all work with On Wed, Feb 24, 2016 at 7:58 AM, Philip Rogers [email protected]
|
|
It works fine for me too. |
progers
added a commit
that referenced
this pull request
Feb 24, 2016
Remove fix that cloned path segments and add tests to show it works
|
In we go! Thank you both. |
jfhenon
added a commit
that referenced
this pull request
Mar 5, 2021
* #issue-fix The new menu will ask if you want to erase the current content but it will ignore the answer * #issue-fix dialog needs to be closer than the original * #issue-fix main menu alignment changes * #issue_fix double click and opensvg issue fixed * #issue-fix process_cancel change to seConfirm * #issue-fix review how the top toolbar display when many buttons are displayed * #issue-fix unwanted css reoved * #issue-fix BOTTOM TOOLS Make sure all features of the bottom toolbar are working * #issue-fix IMPORT IMAGE menu open issue fixed * #issue-fix alert dialog overwrite style * #issue-fix lint issue fixed * npm update + associated fixes * #36 look of opacity button should like the zoom button (without the dropdown button) * #37 Clicking anywhere on the bottom bar (for example below the opacity button is displaying the “color popup”.) * #38 The opacity button does not update with the current element * #42 When you import an image, it works well but the dialog should disappear automatically * Fixes #53: 4th option for the background display fixed * Fixes #53: 4th option for the background fixed * #49 the x button does not work * Fixes #41: Alignment fixes in bottom bar * fix test scenario * #39 opacity button/stroke size/radius button don’t allow the 0 value * #41 The look and alignment of stroke size and stroke style should be consistent with the rest of the bottom bar. * #44 Text font dropdown is broken * #52 Fix the export * Fixes #44: Text font dropdown fixed * Fixes #44: Text font dropdown fixed * Fixes #44: Text font dropdown fixed * Fixes #44: Text font dropdown fixed * Update index.html * update husky * fixes * moves from index.html to js * fix #66 * fix #48 first set of new icons * Reorganize tools in left panel to be more intuitive * Update button styles to adapt to new icons * Fix #48 new set of icons for alignments * Variabilisation of icon bkgd colors Update of color choice to fit new icons design * Update canvas and rulers colors * Improve layer handle design * Modernize SVG Logo but keeping original spirit * Fix #48 continue improve icons * Continue to reorganise left panel * Update right panel handle for layers * Fix #48 new set of icons for main menu * Improve main menu design * Rework menu organisation * Rework menu organisation * Update input element design * New icons * Improve Zoom Module * Improve Color Picker Module * New icons * New icons * #65 restore the feature of start/end marker lines partially did * #64 The export dialog needs to include the quality options (except for PDF) * #39 opacity button/stroke size/radius button don’t allow the 0 value * #39 eslint changes * #69 menu button style overwrite * #65 restore the feature of start/end marker lines * #43 With a small window, the look of the top toolbar is broken * #43 tool top alignment revert * #46 Move this jQuery component to a web component for graduate and picker components * #46 Move this jQuery component to a web component * remove some non standard lint rules * #46 picker convert as pure javascript changes * #46 jquery plugin convert to pure javascript * #46 jquery plugin change to pure javascript * #46 jquery remove and convert to pure javascript * #46 slider issue fixed * #46 ColorValuePicker and js convertion * #46 globals $ remov from slider class * #46 jquery convert pure javascript changes * #46 jquery to js convertion changes * #46 paintbox and current element color set issue fixed * #46 unwanted files removed * #46 $.extend modify changes * #46 extend modifiey changes * #46 $.extend change to pure javascript * #46 extend and data changes * #46 jquery removed * #45 These buttons don't work and dropdown is broken * #45 These buttons don't work and dropdown is broken * #45 These buttons don't work and dropdown is broken commit reverted * #43 With a small window, the look of the top toolbar is broken * Update jQuery.jPicker.js * #76 draggable modification changes * #76 jquery-ui-1.8.17.custom.min.js removed from editor * #76 jquery ui file removed * npm update * #77 console issue fixed * #77 <SVG> button issue fixed * #77 shortkey issue fixed * #77 jquery hotkeys plugin changes * #77 hotkey plugin related changes * #78 hotkey related code comment. * #78 js-hotkeys/jquery.hotkeys.min.js file removed from svgedit.js * #51 Rewrite the color palette without elix * #81 unwanted files removed * #81 folder name renamed * #81 folder rename changes * #81 jquery-ui folder renamed * #81 jquery modify changes * #81 globals $ removed Co-authored-by: Agriya Dev5 <[email protected]> Co-authored-by: mathieucura <[email protected]>
jfhenon
added a commit
that referenced
this pull request
Mar 14, 2021
* #issue-fix The new menu will ask if you want to erase the current content but it will ignore the answer * #issue-fix dialog needs to be closer than the original * #issue-fix main menu alignment changes * #issue_fix double click and opensvg issue fixed * #issue-fix process_cancel change to seConfirm * #issue-fix review how the top toolbar display when many buttons are displayed * #issue-fix unwanted css reoved * #issue-fix BOTTOM TOOLS Make sure all features of the bottom toolbar are working * #issue-fix IMPORT IMAGE menu open issue fixed * #issue-fix alert dialog overwrite style * #issue-fix lint issue fixed * npm update + associated fixes * #36 look of opacity button should like the zoom button (without the dropdown button) * #37 Clicking anywhere on the bottom bar (for example below the opacity button is displaying the “color popup”.) * #38 The opacity button does not update with the current element * #42 When you import an image, it works well but the dialog should disappear automatically * Fixes #53: 4th option for the background display fixed * Fixes #53: 4th option for the background fixed * #49 the x button does not work * Fixes #41: Alignment fixes in bottom bar * fix test scenario * #39 opacity button/stroke size/radius button don’t allow the 0 value * #41 The look and alignment of stroke size and stroke style should be consistent with the rest of the bottom bar. * #44 Text font dropdown is broken * #52 Fix the export * Fixes #44: Text font dropdown fixed * Fixes #44: Text font dropdown fixed * Fixes #44: Text font dropdown fixed * Fixes #44: Text font dropdown fixed * Update index.html * update husky * fixes * moves from index.html to js * fix #66 * fix #48 first set of new icons * Reorganize tools in left panel to be more intuitive * Update button styles to adapt to new icons * Fix #48 new set of icons for alignments * Variabilisation of icon bkgd colors Update of color choice to fit new icons design * Update canvas and rulers colors * Improve layer handle design * Modernize SVG Logo but keeping original spirit * Fix #48 continue improve icons * Continue to reorganise left panel * Update right panel handle for layers * Fix #48 new set of icons for main menu * Improve main menu design * Rework menu organisation * Rework menu organisation * Update input element design * New icons * Improve Zoom Module * Improve Color Picker Module * New icons * New icons * #65 restore the feature of start/end marker lines partially did * #64 The export dialog needs to include the quality options (except for PDF) * #39 opacity button/stroke size/radius button don’t allow the 0 value * #39 eslint changes * #69 menu button style overwrite * #65 restore the feature of start/end marker lines * #43 With a small window, the look of the top toolbar is broken * #43 tool top alignment revert * #46 Move this jQuery component to a web component for graduate and picker components * #46 Move this jQuery component to a web component * remove some non standard lint rules * #46 picker convert as pure javascript changes * #46 jquery plugin convert to pure javascript * #46 jquery plugin change to pure javascript * #46 jquery remove and convert to pure javascript * #46 slider issue fixed * #46 ColorValuePicker and js convertion * #46 globals $ remov from slider class * #46 jquery convert pure javascript changes * #46 jquery to js convertion changes * #46 paintbox and current element color set issue fixed * #46 unwanted files removed * #46 $.extend modify changes * #46 extend modifiey changes * #46 $.extend change to pure javascript * #46 extend and data changes * #46 jquery removed * #45 These buttons don't work and dropdown is broken * #45 These buttons don't work and dropdown is broken * #45 These buttons don't work and dropdown is broken commit reverted * #43 With a small window, the look of the top toolbar is broken * Update jQuery.jPicker.js * #76 draggable modification changes * #76 jquery-ui-1.8.17.custom.min.js removed from editor * #76 jquery ui file removed * npm update * #77 console issue fixed * #77 <SVG> button issue fixed * #77 shortkey issue fixed * #77 jquery hotkeys plugin changes * #77 hotkey plugin related changes * #78 hotkey related code comment. * #78 js-hotkeys/jquery.hotkeys.min.js file removed from svgedit.js * #51 Rewrite the color palette without elix * #81 unwanted files removed * #81 folder name renamed * #81 folder rename changes * #81 jquery-ui folder renamed * #81 jquery modify changes * #81 globals $ removed * remove licence-badger (breaks ci) * build * fix tests * fix folder spelling * skip accessibility for now * Update onpush.yml * use npm dragmove * #82 jquery convertion changes * #82 javascript convertion changes * #82 workarea related javascript convertion changes * #82 javascript convertion cahnges * #82 ungroup button added changes * #82 menu unwanted code removed and button changes * Update .gitignore * refactor with Panel classes * add MainMenu class * move handlers * fix lint * fix test Co-authored-by: Agriya Dev5 <[email protected]> Co-authored-by: mathieucura <[email protected]>
jfhenon
pushed a commit
that referenced
this pull request
Mar 14, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This largely reverts fbbd740 and adds tests (passing in firefox, chrome, and safari) to show svgedit still works. I also manually tested the path tool in each browser and didn't notice any bugs.
I am not sure which bug fbbd740 was fixing but if it crops up again we now have the test infrastructure to better test it.
This fixes #66.