Skip to content

fix #9506: close history panel after select a version #10530

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.

Sign up for GitHub

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

Jump to bottom
Merged
andelf merged 5 commits into from
Feb 2, 2024
Merged

fix #9506: close history panel after select a version #10530

andelf merged 5 commits into from
Feb 2, 2024

Conversation

@CNLHC
Copy link
Contributor

@CNLHC CNLHC commented Nov 22, 2023 •
edited
Loading

This PR is a quick fix to #9506

Since the version list (which is a notification panel) is currently overlapped with the versioned file content (which is a modal), I think the most straightforward way to fix this problem is to close the notification panel after the user open the file content modal.

@andelf andelf self-requested a review November 22, 2023 07:30
@andelf
Copy link
Collaborator

andelf commented Nov 22, 2023

Thanks for the fix.
This breaks the following workflow:

  • Open file history pop-up
  • Check content one by one (now impossible)
  • Find the desired version, then Revert

For a temp fix of #9506, press cmd+= a couple of times.🥹

CC @xyhp915 The file version list and file-specific-version modal should be merged into one.

andelf
andelf reviewed Nov 22, 2023
View reviewed changes
@CNLHC
Copy link
Contributor Author

CNLHC commented Nov 22, 2023

Thanks for the fix. This breaks the following workflow:

  • Open file history pop-up
  • Check content one by one (now impossible)
  • Find the desired version, then Revert

For a temp fix of #9506, press cmd+= a couple of times.🥹

CC @xyhp915 The file version list and file-specific-version modal should be merged into one.

I am willing to try to merge the version list and file-specific-version modal to one modal. Is it ok to submit my solution in this PR?

@andelf andelf requested a review from xyhp915 November 22, 2023 14:00
@xyhp915
Copy link
Collaborator

xyhp915 commented Nov 30, 2023

For a temp fix of #9506, press cmd+= a couple of times.🥹
CC @xyhp915 The file version list and file-specific-version modal should be merged into one.

I am willing to try to merge the version list and file-specific-version modal to one modal. Is it ok to submit my solution in this PR?

So appreciate that!

@CNLHC
Copy link
Contributor Author

CNLHC commented Nov 30, 2023

For a temp fix of #9506, press cmd+= a couple of times.🥹
CC @xyhp915 The file version list and file-specific-version modal should be merged into one.

I am willing to try to merge the version list and file-specific-version modal to one modal. Is it ok to submit my solution in this PR?

So appreciate that!

Ok, I will try to submit my solution to this PR

@CNLHC
Copy link
Contributor Author

CNLHC commented Nov 30, 2023

Here is how the version select works after e876a15

logseq-pr-1.mp4

@CNLHC CNLHC changed the title fix: close history panel after select a version fix: close history panel after select a version #9506 Nov 30, 2023
@CNLHC CNLHC changed the title fix: close history panel after select a version #9506 fix #9506: close history panel after select a version Nov 30, 2023
@CNLHC
Copy link
Contributor Author

CNLHC commented Nov 30, 2023

BTW，I proposed a new feature request (a diff view) in the forum which is related to this PR.

@CNLHC CNLHC force-pushed the feat/close-notifications-after-select-version branch from c038711 to 0b762cd Compare November 30, 2023 11:39
andelf
andelf approved these changes Dec 7, 2023
View reviewed changes
Copy link
Collaborator

@andelf andelf left a comment

Choose a reason for hiding this comment

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

LGTM

@CNLHC CNLHC force-pushed the feat/close-notifications-after-select-version branch from 0b762cd to c2a371c Compare December 11, 2023 12:25
@tiensonqin tiensonqin force-pushed the feat/close-notifications-after-select-version branch from 955637a to 483f477 Compare December 28, 2023 12:19
@andelf andelf self-assigned this Jan 19, 2024
@andelf
Copy link
Collaborator

andelf commented Feb 2, 2024

@CNLHC I'll handle the remaining fix to get it merged.

@andelf andelf force-pushed the feat/close-notifications-after-select-version branch from be23411 to 38621a4 Compare February 2, 2024 08:38
@andelf andelf force-pushed the feat/close-notifications-after-select-version branch from 38621a4 to fa48f37 Compare February 2, 2024 09:08
@andelf andelf merged commit c3df737 into logseq:master Feb 2, 2024
@turtaf turtaf mentioned this pull request Feb 8, 2024
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andelf andelf andelf approved these changes

@xyhp915 xyhp915 Awaiting requested review from xyhp915

Assignees

@andelf andelf

Labels

feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@CNLHC @andelf @xyhp915