ImpulseAdventure / GUIslice-Builder

Cross-platform drag & drop GUI builder for GUIslice
Other
163 stars 35 forks source link

Scale, move widgets with mouse #255

Closed etet100 closed 1 month ago

etet100 commented 2 months ago

The thing I strongly miss in the interface is resizing widgets with the mouse. I don't have much experience so I'd appreciate any tips or fixes. This is a draft and has bugs. For example, it works incorrectly with a scale other than 1.

image

Brief demonstration:

https://youtu.be/AImc7qOuqes

Another improvement is that page zoom can be controlled by the mouse wheel. I have also added zoom reset to menu and ribbon.

image

Pconti31 commented 2 months ago

@etet100 Looks very interesting. I'll take a look at your code and post if I notice anything. Are there any other issues beside scale of 1? Paul--

etet100 commented 2 months ago

I still see plenty of problems, but let's say they are mostly not complete blockers. There is a lack of size/position constraints. Widgets like lines are hard to grab. When moving close to the edge of the page, the widget locks up (this is due to the limitations of the existing solution). Snap to grid doesn't quite work.

I still have a lot of work left to do.

Pconti31 commented 2 months ago

@etet100 Still, A really nice job.

Pconti31 commented 2 months ago

@etet100 Overall I think you have done an excellent job to this point so don't take any of this to heart. Must be hard to follow someone elses code without any real documentation.

In your ResizeCommand class I'm not sure if use of model.setX and model.setY is ok or not. You should test with undo.

But calling setWidth() and setHeight() directly will cause crashes. Test with resizing checkbox and radio buttons. It should be: model.setWidth(newSnappedSize); model.changeValueAt(Integer.valueOf(newSnappedSize), WidgetModel.PROP_WIDTH);

model.setHeight(newSize); model.changeValueAt(Integer.valueOf(newSize), WidgetModel.PROP_HEIGHT);

changeValueAt is the basis of my undo commands and most of the logic to prevent crashes dealing with varaibles out of bounds or not the class we think they should be. Java swing has a habit of placing strings where we want integers for example, changeValueAt() will take care of a lot of details for you...

Paul--

etet100 commented 2 months ago

I already noticed many of those problems when I was working on Line support. To be honest, I'm not a fan of some of the solutions used here. For example, line object (which has no height/width but length) inherits from Widget and in a rather unclear way overrides some properties (reusing PROP_* indexes) but still alows to use old setters and getters. Which leads to the crashes you mentioned above. I keep wondering how to finish it the right way.

Pconti31 commented 2 months ago

@etet100 For line widget can't you simply override Widget class inside LineWidget.java Inside LineWidget.java

  @Override
  public void drawSelRect(Graphics2D g2d, Rectangle b) {
    if (!bSelected) {
      return;
    }
    ...
    if (m.isVertical()) {
      // support and display handles top and bottom
    } else {
      // support and display handles left and right
    }

Of course, you will also need to modify ResizeCommand.java I support line because of peoples requests for single pixel line. Otherwise thet could just use Box. Paul--

etet100 commented 2 months ago

Ok. I still have a lot of ideas but I wouldn't want to do everything at once. For now, I'm happy with what I've done.

I made some changes that may be unpopular, but in my opinion were necessary.

Now I will need help with testing.

Pconti31 commented 2 months ago

@etet100 I'll do some testing myself.
I would like a full list of the new features. Please send a simple text file to p_conti@hotmail.com The README.md and user_guide.md will also need updating before a merge can take place. I believe my CONTRIBUTING.md mentions user_guide but I forgot the README.

As I made some changes that may be unpopular, but in my opinion were necessary. Again feel free to contact me before doing anything you think could be viewed as problematic so you don't waste your time on something I'm not going to merge.

NOTE: To avoid problems in the future it's considered best practice to implement one feature for each pull request. This makes it easier for the person doing the approval (me or Calvin) and avoids hopefully any hard feelings on the part the person nice enough to have spent the energy doing the implemention.

For example, All code changes for the resize of widgets are one feature while the mods to the toolbar would have been a second one, and button changes another. Not that I have seen anything so far that I would object too. This avoid any headaches of what features are going to allowed or not. I'll update the CONTRIBUTING.md to reflect this.

Thank you for your work on this. Paul-

etet100 commented 2 months ago

In this case, my main ‘feature’ was to ‘make it more user friendly’. I would not be able to work on each part separately. I needed each of the changes and I needed to have them in the same branch. If my changes are not accepted then I will just work on my fork.

etet100 commented 2 months ago

As for the documentation: it's hard to imagine editing every screenshot. Especially if adding another feature (which I've already thought about) might require further changes. This documentation is TOO long and detailed. For example, why describe vertical and horizontal alignment separately? Icons/names should be self-descriptive as much as possible. We also cannot assume that the user has never dealt with computers or similar apps. Maintaining such a long document is a full-time task.

Pconti31 commented 2 months ago

@etet100 For the user guide only a sentence ot two is needed in section 2.3 Creating your UI to indicate that users can select the red handles to drag and resize widgets. In Software Engineering the coding isn't complete until the documentation is done. Since you seem unwilling to do so and since the main update is so useful I'll take care of docs before doing a release.

But it would be useful if you send me a text file with a complete list of the new functionality.

If you send directly to my external email account p_conti@hotmail.com then I'll have your email address so I can report bugs I fine when testing your version without bogging down this pull request trail. For example, using ex02_bld_btn_txt you can't select the text button contained inside a Box element.

As my main ‘feature’ was to ‘make it more user friendly’ that is always subjective and still could easily be broken up into mutiple smaller pull requests. Again resizing being one pull and various toolbar changes another. Once again I haven't seen anything I object too. But if you ever make pull requests to someone elses repository you might want to this in mind to keep a good open source reputation. Paul--

Pconti31 commented 2 months ago

@etet100 In the Limited Testing I have done things look very good. Thanks again for all your hard work!

Pconti31 commented 2 months ago

@etet100 FYI, if you restore PagePane.findOne() back to its original form the elements with other elements selection will work again thus fixing ex02_bld_btn_txt among many others. Paul--

etet100 commented 2 months ago

You are right. I should start checking widgets from the last one.

Pconti31 commented 1 month ago

@etet100 Testing has gone well with me. Other than the view\editor panel not having snap to grid disable icon showing and minor mods needed to view pull-down adding snap to grid and disable icon for snap to grid. I haven't found anything else wrong. I can finish these things up after doing a merge.
Is there anything you know of that should prevent me from doing this merge? Paul--

etet100 commented 1 month ago

I have not noticed any problems. Anyway, we can always make improvements. Even after merge.