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

PoC - FlyoutPaletteComposite with custom area #692

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ptziegler
Copy link
Contributor

No description provided.

@ptziegler
Copy link
Contributor Author

#685

@ptziegler ptziegler force-pushed the flyout-palette-composite-with-custom-area branch from edd5aa0 to 2ec6a8c Compare February 8, 2025 19:16
@feilimb
Copy link
Contributor

feilimb commented Feb 10, 2025

Hi Patrick,

I got a chance to try this out and in effect compare it to our existing implementation, where we render an 'overview ruler' between the palette and the main editor window.

Some observations / differences in behaviour:

  • 'custom area' component in our implementation remains shown when palette is collapsed, whilst it is hidden when integrated with this PR changeset (an example screenshot for this case only is provided at end of this message)
  • 'custom area' component in our implementation remains on the right hand side, when the palette is docked on the Left, whereas with this PR changeset it moves over to the left
  • 'custom area' component in our implementation remains shown on right of editor view area, when the palette is undocked entirely/moved to its own view, whereas with this PR changeset it becomes hidden when the palette is undocked/moved to a view

I'm starting to think that the behaviours we have to date are making this more 'bespoke' than a general 'custom area' and that the behaviours are very tied to the fact that the component represents an overview ruler, which is always rendered on the right hand side, regardless of palette dock position / undocked state.

I'm not sure where that leaves us as such, but perhaps in the direction of attempting to add specific support for provision of an overview ruler - but I'm not sure that you / GEF team want to go down that route.

If it is useful at all, I can share the layout algorithm I have in our bespoke implementation (reflecting the behaviours I mentioned above). It is essentially the contents of the layout(boolean) method and the two private layoutEast and layoutWest type methods.

Example of first bullet point above:
image

@ptziegler
Copy link
Contributor Author

But in that case I have to agree with @azoitl... To me, this sounds like you want to have the ruler be part of the viewer and not the palette. Especially when e.g. the ruler should remain when the palette is detached.

@feilimb
Copy link
Contributor

feilimb commented Feb 10, 2025

But in that case I have to agree with @azoitl... To me, this sounds like you want to have the ruler be part of the viewer and not the palette. Especially when e.g. the ruler should remain when the palette is detached.

Yes I think you are right, I added a comment in the other thread, pasting it here now:

I guess with our current bespoke implementation we are 'piggy-backing' on the palette composite to get an overview ruler to appear, but I am starting to think now that perhaps the ability to create a custom area (ie. overview ruler) which visually appears between the editor area and the palette, could perhaps come from the class above - which creates the palette composite, ie 'GraphicalEditorWithFlyoutPalette' or something along those lines.

Whilst we use the 'GraphicalEditorWithFlyoutPalette' code as a basis for our own version of that class, do you think it would be useful to have a change within 'GraphicalEditorWithFlyoutPalette' itself, which provides a means to have an overview ruler component placed between the editor view and the palette composite?

I have a hunch that this approach may have been attempted many years ago on our side, but for some reason there was issues with it, but I can't recall now what those issues were, and perhaps they are no longer relevant.

@ptziegler
Copy link
Contributor Author

ptziegler commented Feb 10, 2025

Whilst we use the 'GraphicalEditorWithFlyoutPalette' code as a basis for our own version of that class, do you think it would be useful to have a change within 'GraphicalEditorWithFlyoutPalette' itself, which provides a means to have an overview ruler component placed between the editor view and the palette composite?

No. As the documentation of the GraphicalEditorWithFlyoutPalette states, it is merely a template. Any customizations should be made in copies or subclasses, but not in the template itself.

My suggestion is to do something similar to what is done in the LogicEditor; To override the createGraphicalViewer() and getGraphicalControl() methods. In the first method, you create a composite that contains both the control for the viewer and your ruler. This composite is then returned by the second method.

@Override
protected void createGraphicalViewer(Composite parent) {
rulerComp = new RulerComposite(parent, SWT.NONE);
super.createGraphicalViewer(rulerComp);
rulerComp.setGraphicalViewer((ScrollingGraphicalViewer) getGraphicalViewer());
}

@Override
protected Control getGraphicalControl() {
return rulerComp;
}

In the example those rulers are put to the northern and western side of the editor but in your own implementation, you are free to put them wherever you want.

image

@feilimb
Copy link
Contributor

feilimb commented Feb 10, 2025

Thanks Patrick, that sounds like the way forward with this stuff and sorry for all the time spent thus far when it looks like the overview ruler should never have been in the palette to begin with!

I will try the approach suggested above when I get time.

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

Successfully merging this pull request may close these issues.

2 participants