search

origo-map / draw-plugin

Draw tool plugin for Origo
MIT License
1 stars 4 forks source link

Adds freemode, disable featureinfo on active #8

Closed Bimane1900 closed 4 years ago

Bimane1900 commented 5 years ago

This adds a toggle-button to toggle between drawing lines and polygons with freehand-mode or not, and disables featureinfo and pinning while the drawtool is active.
Also as stated in a comment on commit, this fixes bugs related to adding text-type and adds tooltips.

freemode-draw

tonnyandersson commented 4 years ago

@Bimane1900, @Grammostola

The freehand mode is a cool feature! Nevertheless, I have a few remarks regarding the implementation.

First of all, this PR should have been divided into several PR's, each addressing a separate issue. That makes it easier to address, test and make changes to each individual fix. It also makes tracking changes easier. Not a big deal, just mentioning it for future reference. Oh, there should be a corresponding issue, as well, preceding the PR. Or several issues, actually.

Props for fixing the featureinfo and pinning bug. Though, I'm not sure those features should be disabled when activating the draw toolbox. I think it can be useful to have the ability to show feature info, at least, even in draw mode. But those features should definitely be disabled when activating a draw tool. EDIT: Actually, I can accept this. For now. :)

What's up with that fly-in transition when activating the toolbox? Feels like a PowerPoint presentation from 2003. I guess the purpose is to draw attention to the toolbox, but it is a bit...blunt. Please speed it up (0.2s works better) or remove it completely. The latter is preferred, in my opinion. You are, of course, free to keep the animation in your local implementation.

That's all I've got for now. @johnnyblasta, any opinions?

johnnyblasta commented 4 years ago

Tested and it seems to work as intended. Some linter errors in drawhandler.js and drawtoolbar.js that could be easily fixed.

Grammostola commented 4 years ago

Thanks for the feedback. (New PRs will follow the described course)

Hahaha, Powerpoint presentation from 2003 : ) We saw a need for something to call attention to the draw toolbar appearing during usability testing. It's now been speeded up and drawhandler.js and drawtoolbar.js have experienced linting.

(If this is not acceptable we can create several issues and wait for classification then try and file matching PRs)

tonnyandersson commented 4 years ago

Nice!