Skip to content

[6295] Make it possible to hide the minimap by default#6305

Merged
frouene merged 1 commit intofba/doc/minimapShapfrom
fba/enh/minimap
Mar 27, 2026
Merged

[6295] Make it possible to hide the minimap by default#6305
frouene merged 1 commit intofba/doc/minimapShapfrom
fba/enh/minimap

Conversation

@florianbarbin
Copy link
Copy Markdown
Contributor

  • The specifier can choose to hide the minimap by default for a specific diagram description
  • The minimap hide/reveal action is now persisted for a specific diagram.

Bug: #6295

Pull request template

General purpose

What is the main goal of this pull request?

  • Bug fixes
  • New features
  • Documentation
  • Cleanup
  • Tests
  • Build / releng

Project management

  • Has the pull request been added to the relevant project and milestone? (Only if you know that your work is part of a specific iteration such as the current one)
  • Have the priority: and pr: labels been added to the pull request? (In case of doubt, start with the labels priority: low and pr: to review later)
  • Have the relevant issues been added to the pull request?
  • Have the relevant labels been added to the issues? (area:, difficulty:, type:)
  • Have the relevant issues been added to the same project and milestone as the pull request?
  • Has the CHANGELOG.adoc been updated to reference the relevant issues?
  • Have the relevant API breaks been described in the CHANGELOG.adoc? (Including changes in the GraphQL API)
  • In case of a change with a visual impact, are there any screenshots in the CHANGELOG.adoc? For example in doc/screenshots/2022.5.0-my-new-feature.png

Architectural decision records (ADR)

  • Does the title of the commit contributing the ADR start with [doc]?
  • Are the ADRs mentioned in the relevant section of the CHANGELOG.adoc?

Dependencies

  • Are the new / upgraded dependencies mentioned in the relevant section of the CHANGELOG.adoc?
  • Are the new dependencies justified in the CHANGELOG.adoc?

Frontend

This section is not relevant if your contribution does not come with changes to the frontend.

General purpose

  • Is the code properly tested? (Plain old JavaScript tests for business code and tests based on React Testing Library for the components)

Typing

We need to improve the typing of our code, as such, we require every contribution to come with proper TypeScript typing for both changes contributing new files and those modifying existing files.
Please ensure that the following statements are true for each file created or modified (this may require you to improve code outside of your contribution).

  • Variables have a proper type
  • Functions’ arguments have a proper type
  • Functions’ return type are specified
  • Hooks are properly typed:
    • useMutation<DATA_TYPE, VARIABLE_TYPE>(…)
    • useQuery<DATA_TYPE, VARIABLE_TYPE>(…)
    • useSubscription<DATA_TYPE, VARIABLE_TYPE>(…)
    • useMachine<CONTEXT_TYPE, EVENTS_TYPE>(…)
    • useState<STATE_TYPE>(…)
  • All components have a proper typing for their props
  • No useless optional chaining with ?. (if the GraphQL API specifies that a field cannot be null, do not treat it has potentially null for example)
  • Nullable values have a proper type (for example let diagram: Diagram | null = null;)

Backend

This section is not relevant if your contribution does not come with changes to the backend.

General purpose

  • Are all the event handlers tested?
  • Are the event processor tested?
  • Is the business code (services) tested?
  • Are diagram layout changes tested?

Architecture

  • Are data structure classes properly separated from behavioral classes?
  • Are all the relevant fields final?
  • Is any data structure mutable? If so, please write a comment indicating why
  • Are behavioral classes either stateless or side effect free?

Review

How to test this PR?

Please describe here the various use cases to test this pull request

  • Has the Kiwi TCMS test suite been updated with tests for this contribution?

@florianbarbin florianbarbin added this to the 2026.5.0 milestone Mar 18, 2026
@florianbarbin florianbarbin force-pushed the fba/enh/minimap branch 2 times, most recently from 0e5490b to 035c8d6 Compare March 18, 2026 17:07
@florianbarbin florianbarbin linked an issue Mar 18, 2026 that may be closed by this pull request
*
* @author fbarbin
*/
public record ShowMinimapSuccessPayload(UUID id, Boolean showMinimap) implements IPayload {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be a Boolean rather than a boolean ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

public void handle(One<IPayload> payloadSink, Many<ChangeDescription> changeDescriptionSink, IEditingContext editingContext, DiagramContext diagramContext, IDiagramInput diagramInput) {
this.counter.increment();

String message = this.messageService.invalidInput(diagramInput.getClass().getSimpleName(), GetToolbarInput.class.getSimpleName());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong input in the error message

Copy link
Copy Markdown
Contributor Author

@florianbarbin florianbarbin Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed, copy paste issue :D

VariableManager variableManager = new VariableManager();
variableManager.put(VariableManager.SELF, targetElement);
variableManager.put(IEditingContext.EDITING_CONTEXT, editingContext);
variableManager.put(DiagramContext.DIAGRAM_CONTEXT, diagramContext);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this variableManager for? It's not an expression that can be evaluated.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed.

* @author fbarbin
*/
@Service
public class ShowMinimapProvider implements IShowMinimapProvider {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implementation is specific for the view description, it should be mentioned in its name.
By the way what happen if it's not a view description, I think that this code will not work.
I think we need to add a canHandle method to the interface

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done: I added a canHandle on the service and the EventHandler take a list of IShowMinimapProvider and use the first one that can handle.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name of this class still doesn't make it clear that it's meant to handle descriptions from the view.
I think it might be a good idea to specify that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something like DiagramDescriptionViewShowMinimapProvider ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even just ViewShowMinimapProvider is enough for me

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@florianbarbin florianbarbin changed the base branch from fba/doc/minimapShap to cooldown March 19, 2026 08:24
@florianbarbin florianbarbin changed the base branch from cooldown to fba/doc/minimapShap March 19, 2026 08:26

//4. We switch to the initial Topography diagram
const representationItem = await explorer.getTreeItemLabel('Topography');
representationItem.click();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

await missing

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

await expect(page.locator('.react-flow__minimap')).toBeAttached();

//4. We switch to the initial Topography diagram
const representationItem = await explorer.getTreeItemLabel('Topography');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be done with explorer.select

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@florianbarbin florianbarbin requested a review from frouene March 19, 2026 09:30
@florianbarbin florianbarbin force-pushed the fba/enh/minimap branch 2 times, most recently from 25d9a82 to b76394f Compare March 20, 2026 09:00
DiagramDescription diagramDescription = optionalDiagramDescription.get();
var optionalShowMinimapProvider = this.showMinimapProviders.stream()
.filter(iShowMinimapProvider -> iShowMinimapProvider.canHandle(editingContext, diagramContext, diagramDescription))
.findFirst();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To test the functionality, I wanted to create a simple scenario where the minimap is only displayed once there is at least one node on the diagram. The problem with this approach (where we simply look for the first provider that returns true) is that, depending on the order in which Spring scans the beans, we might end up having to skip beans in order to implement a new mechanism.
I wonder if it wouldn’t be better to take all providers with canHandle set to true and aggregate all the booleans (using an and operator) to get a consolidated value.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have changed the code to perform a reduce operation (with and operator) with each providers that can handle the request

- The specifier can choose to hide the minimap by default for a specific diagram description
- The minimap hide/reveal action is now persisted for a specific diagram.

Bug: #6295
Signed-off-by: Florian Barbin <florian.barbin@obeosoft.com>
Signed-off-by: Florian ROUËNÉ <florian.rouene@obeosoft.com>
@frouene
Copy link
Copy Markdown
Contributor

frouene commented Mar 26, 2026

I just moved the entry in the changelog to be in the 2026.3.0 section.
Can me merged after build

@frouene frouene merged commit 10004fb into fba/doc/minimapShap Mar 27, 2026
4 of 5 checks passed
@frouene frouene deleted the fba/enh/minimap branch March 27, 2026 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add the ability to hide the mini map by default

2 participants