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.

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

New GUI, new tools, new features... #31

Open
rdbeach opened this issue Apr 15, 2020 · 28 comments
Open

New GUI, new tools, new features... #31

rdbeach opened this issue Apr 15, 2020 · 28 comments

Comments

@rdbeach
Copy link
Contributor

rdbeach commented Apr 15, 2020

Hey. Thanks for a great board. I have forked this board and made some alterations (actually quite a bit), mostly for my own personal use as a math tutor. Check them out at rdbeach/wb So far, the changes have undergone limited testing on Safari and Chrome, both on my laptop and on my phone. Seems to work pretty good so far.

A list of changes:

Redesign of the GUI/layout
Added whiteout tool
Added circle tool
Modified text tool so that the input appears closer to the text
Made the removal tool more robust so you can remove small objects easier
Added an optional grid
Added an eraser tool
Added a clear board tool
Added an undo tool
Added a redo tool
Added a zoom in and zoom out tool in separate files.
Made some changes to the server to accommodate the new tools.

@lovasoa
Copy link
Owner

lovasoa commented Apr 15, 2020

Your work is great ! Congratulations @rdbeach !

I am reposting here the comment I made on your commit :

Hello @rdbeach !
I stumbled on this commit on my github feed. It looks like you've done some great work on WBO ! 👍 Would you be interested by working with me on integrating some of these changes back into WBO ? This would give them more visibility, reduce the maintenance burden on your fork, and potentially enable some more contributions by other parties. I am especially interested by your new eraser tool. Working with svg masks is a great idea that I wish I had from the beginning !
There are some browser compatibility and other issues with your changes, though, and I would be glad to work with you on solving these.
Cheers,
@lovasoa

Also, I think you forgot to commit some files. I am not seeing the eraser tool, and the undo/redo don't seem to work... The internationalization seems to be broken too...
It would be awesome if we could work together on polishing and integrating these changes.

image

@rdbeach
Copy link
Contributor Author

rdbeach commented Apr 15, 2020 via email

@lovasoa
Copy link
Owner

lovasoa commented Apr 15, 2020

There is currently a single big commit. It would be good to make several small pull requests that we can discuss and work on independently.
Some of them can already be merged now, such as

  • GUI/layout improvements
  • the circle tool
  • Modified text tool so that the input appears closer to the text
  • Made the removal tool more robust so you can remove small objects easier
  • Added an optional grid

@rdbeach
Copy link
Contributor Author

rdbeach commented Apr 15, 2020

How do you break a pull request into smaller ones?

Maybe make a branch for different features? I can fill out the minimal code changes for each branch and we can review and discuss. But maybe just one branch at a time to keep things simpler.

@lovasoa
Copy link
Owner

lovasoa commented Apr 16, 2020

Yes, you are right, doing the PRs one by one will avoid conflicts and keep things simpler. Just create a branch with the changes and open a PR from that branch to master on lovasoa/whitebophir. Maybe let's start with the smalleest and least controversial things, like the circle tool or the change in the position of the text input field.

@rdbeach
Copy link
Contributor Author

rdbeach commented Apr 21, 2020

Hi. Quick update. I added a transform tool, and an area eraser. I'm actually using the board right now with some of my clients, and I will probably do some more rigorous testing of it in the days to come. I've been doing this partly for my work, and partly as a hobby to keep me busy during this difficult time. Unfortunately, I've made so many changes to the original board that I don't know if its feasible (or worth the effort) to merge them at this point. One more feature I may want to incorporate is loading a background image.

@lovasoa
Copy link
Owner

lovasoa commented Apr 21, 2020

A background image tool was implemented in #21

I will look into merging your work as it is tested. Do you plan to support firefox ?

@rdbeach
Copy link
Contributor Author

rdbeach commented Apr 22, 2020

I just downloaded the latest Firefox a few hours ago. At first, it looked like it wasn't working at all, but it turned out to be just a few minor glitches, and now it looks good, as far as I can tell. Should I support edge as well? Haven't tried that one.

@finnboeger
Copy link
Contributor

finnboeger commented Apr 26, 2020

Hi I had a go at starting to split the changes into smaller more manageable chunks.
For now I have split some inital changes to the board initialization and the server socket handling off into base-changes and the gui changes into style-changes.
The code for the improved removal tool has been split off to tool/eraser, however it - like the future branches I will create - depends on the first two changesets.

The next steps that I can see are

  • The circle tool
  • The image tool
  • The text changes
  • The cursor - this massively increases the amount of messages being sent, should possibly be throttled
  • The transform tool
  • The touch-up eraser, which requires adding support for layers to every tool
  • The grid
  • The separate zoom buttons
  • The undo, redo and clear tools which require a server side undo/redo history and new message type. Also add a confirmation dialog to the clear button and possibly restrict it
  • > A UX improvement in conjunction with the 3 above: implement them as simple buttons instead of tools, reducing the amount of clicks necessary to e.g. toggle the grid

@lovasoa lovasoa mentioned this issue Apr 26, 2020
@lovasoa
Copy link
Owner

lovasoa commented Apr 26, 2020

Great, let's discuss that in #38 and #39 ! Thank you for taking the time to split your work in chunks, it will make it easier to integrate.

lovasoa added a commit that referenced this issue Apr 26, 2020
Thanks a lot and congratulations to @finnboeger and @rdbeach !
@lovasoa lovasoa changed the title Not really an issue but... New GUI, new tools, new features... Apr 26, 2020
@rdbeach
Copy link
Contributor Author

rdbeach commented Apr 27, 2020

"The touch-up eraser, which requires adding support for layers to every tool"

Also, the transform tool requires each drawing element to set their transform property.

@rdbeach
Copy link
Contributor Author

rdbeach commented Apr 27, 2020

BTW has anyone discussed client configuration? I see that you updated the backend configuration. Probably when you start putting in so many features, not everyone wants or needs them. Maybe another config file for the front end?

@finnboeger
Copy link
Contributor

I realized that part about the transform tool when I had a closer look at it, thats why I'm holding off on implementing it until the other tools have been merged.

As for front end configuration I have added your latest changes to the cursor to allow disabling it, but there is no settings page yet. I believe adding a oneTouch tool that would open a setting modal might be the best way to go?

I would also like to have server and board wide settings to allow you to disable certain features for all boards or on board creation, e.g. you might not want the document upload feature on a public server or the cursors on a low power server or very high use board.

@lovasoa
Copy link
Owner

lovasoa commented Apr 27, 2020

Yes, having the configuration reflected on the client is probably a good idea. It could be done the same way as it is for translations.

Also, what would you think about switching to using modern javascript, and a transpiler ? This would make the client-side code more readable in many instances.

@finnboeger
Copy link
Contributor

I definitely see the benefit however I have no experience setting it up 😆
We use typescript at work but I believe there would be quite a bit of effort in getting everything ready for such a switch.

@lovasoa
Copy link
Owner

lovasoa commented Apr 27, 2020

I can set it up. It will just take some time, and you will probably have to rebase your PRs. Let's finish the "easy" PRs, and then I'll get to work. What we can merge without much changes are your current PRs for :

  • circle tool
  • improved text tool
  • cursor

I added comments on those.

@finnboeger
Copy link
Contributor

Another question for my testing: Which browsers should we support?
I am currently only testing in Chrome and Firefox as I am on Linux and don't have any macOS or iOS devices to test on them.
I can perform tests in Internet Explorer if necessary but I would much rather let it die. As the new Edge is based on Chromium as well I believe it should automatically be supported.

@finnboeger
Copy link
Contributor

finnboeger commented Apr 27, 2020

When integrating all the currently proposed features I'd recommend a new Menu structure:
wbo menu
The icons are obviously not final and we'd have to rethink how we handle hovers (maybe ignore hover over the tooltip and only attach the hover event to the icon itself).
It is also currently missing the touch up eraser as I'm not sure where to put it without creating too much confusion or incentivising its use too much

What do you think?

PS: I just realized I forgot the whiteout tool which I would keep as the secondary function of the pencil and the settings which I would keep as a single column below the tools. We probably also need a setting option that brings up a pop up menu with the different configuration options

@rdbeach
Copy link
Contributor Author

rdbeach commented Apr 28, 2020

Right now I am testing on Chrome, Safari, and Firefox. Recent versions only. Also I have tested it on safari and chrome for iPhone. Have not tested android phones. I will most likely continue to test daily, especially since I am using it with my clients.

@lovasoa
Copy link
Owner

lovasoa commented Apr 28, 2020

@finnboeger : 👍 to the two-columns menu on desktop. We'll have to see if it doesn't take too much space on small screens.

@rdbeach : WBO was started a long time ago, and it was initially designed to work on internet explorer 9+ too. However, I don't test it on IE on a regular basis...

@finnboeger
Copy link
Contributor

Regardless whether we make the change to the two-column layout we should offset the canvas to the right by the width of the menu. Otherwise if you e.g. write something and then zoom out it gets occluded by the menu.

@lovasoa
Copy link
Owner

lovasoa commented Apr 29, 2020

What about adding a button to hide the menu instead ? On small screens, I think we cannot afford to reduce the size of the canvas...

@finnboeger
Copy link
Contributor

Very good point. How about on small devices we hide the menu by default and only display the currently active tool and maybe the size indicator (using the old UI where it showed a dot of the current size but with the current color and opacity)?
When clicking on the tool it would open up an overlay with the available tools whereas clicking on the size indicator would bring up a combined menu for changing size, color and opacity.

Tying into that I noticed another UX issue in my last lecture:
New Users don't know about the available functionality. I believe adding a screen explaining all available features for users that are opening it for the first time would be helpful. That could also explain how to select their tool for mobile users.
I have created a couple of mockups:

Mobile UI:
mobileUI

Mobile UI hidden:
mobileUIhidden

Mobile UI expanded:
mobileUIopen

Intro / Help Screen:
wbo intro

I added a whole load of features in the mockups but I don't expect that we will be able to implement them all directly.

What do you think?

@tomichGIT
Copy link

hi guys, thanks for this amazing script. I have a question do... if you are working together, should we be using @lovasoa or @rdbeach github repo? i loved the changes made by @rdbeach for teaching math.. but i dont want to stick with an unopdated version. Are you guys merging into one? thanks a lot again!

@lovasoa
Copy link
Owner

lovasoa commented May 11, 2020

Hello! Yes, a lot of the changes are being merged, have a look at the pull requests! @finnboeger is doing a fantastic job at splitting the features into distinct PRs, fixing bugs, making the code cleaner and more maintainable. This represents a decent amount of work, so if you want to contribute on the existing pull requests, please do!

@rdbeach
Copy link
Contributor Author

rdbeach commented May 13, 2020

Hey, what's going on over here? I see you've made some changes. Where did @finnboeger go? I'm almost done with the buildout now. Maybe we can work together to clean up the code a bit. If you want to take it to the next level, we should make an organizational account and put the repo in there, where we can work as collaborators. It would probably attract more interest as well, because, then, it's not like you're just contributing to someone's pet project.

@lovasoa
Copy link
Owner

lovasoa commented May 13, 2020

Hello ! Good that you are almost done with your changes, we'll be able to integrate more of them ! I think there is quite some work, so feel free to open new PRs and comment on the ones of @finnboeger. Ideally, we'll keep one feature by PR, to keep things manageable. We can create an organization, but if you're okay with it, I'd still like to review each change individually, and avoid giving external commit access immediately.

@mwllgr
Copy link

mwllgr commented May 29, 2021

Hi, any news on these changes? The grid and dashed functions would be really great to have. @lovasoa @rdbeach

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants