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

fix(web-pkg): hide sidebar editor action when the editor is already opened #12110

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

Conversation

LukasHirt
Copy link
Collaborator

Description

When an editor is already opened and the user clicks on the Open in ... action, it pushes the same editor route into the history causing a bug where the user needs to click twice on the close button to actually close the editor. This change hides the editor action if an editor with matching route name is already opened which prevents this behaviour.

Related Issue

Motivation and Context

No confusion with re-opening the active editor.

How Has This Been Tested?

  • test environment: chrome & 🤖
  • test case 1: added unit test
  • test case 2: open an image in the preview app and open sidebar

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests
  • Documentation
  • Maintenance (e.g. dependency updates or tooling)

@LukasHirt LukasHirt added the Category:Enhancement Add new functionality label Jan 17, 2025
@LukasHirt LukasHirt self-assigned this Jan 17, 2025
@LukasHirt LukasHirt requested a review from saw-jan January 17, 2025 10:02
@LukasHirt LukasHirt mentioned this pull request Jan 17, 2025
10 tasks
…pened

When an editor is already opened and the user clicks on the Open in ... action, it pushes the same editor route into the history causing a bug where the user needs to click twice on the close button to actually close the editor. This change hides the editor action if an editor with matching route name is already opened which prevents this behaviour.
@LukasHirt LukasHirt force-pushed the fix(web-pkg)/hide-sidebar-action branch from fe1a35b to dea4bbb Compare January 17, 2025 10:21
@LukasHirt LukasHirt enabled auto-merge January 17, 2025 10:36
@nabim777
Copy link
Member

I have tested this bug-fix PR and found that it introduced the following changes:

  • The preview button has been replaced with Open in Collabora.

Bug found in this PR

@LukasHirt When we open the image in Collabora and try to close it, the UI becomes unresponsive:

Screencast.from.01-17-2025.05.02.09.PM.webm

@saw-jan
Copy link
Member

saw-jan commented Jan 17, 2025

I have tested this bug-fix PR and found that it introduced the following changes:

  • The preview button has been replaced with Open in Collabora.

Bug found in this PR

@LukasHirt When we open the image in Collabora and try to close it, the UI becomes unresponsive:

Screencast.from.01-17-2025.05.02.09.PM.webm

This should be fixed by #12112

@nabim777
Copy link
Member

I have tested this bug-fix PR and found that it introduced the following changes:

  • The preview button has been replaced with Open in Collabora.

Bug found in this PR

@LukasHirt When we open the image in Collabora and try to close it, the UI becomes unresponsive:
Screencast.from.01-17-2025.05.02.09.PM.webm

This should be fixed by #12112

Yes, it has been fixed by PR, and I have tested it.

@LukasHirt
Copy link
Collaborator Author

This will need some more work as the routeName is not always defined leading into wrong behaviour of hiding the action even outside of the editor. Since the regression has been fixed in #12112, I would suggest postponing this into next release.

@LukasHirt LukasHirt marked this pull request as draft January 20, 2025 09:12
auto-merge was automatically disabled January 20, 2025 09:12

Pull request was converted to draft

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category:Enhancement Add new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants