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

Feature: Move open with to main menu when right clicking recent files #11304

Merged
merged 11 commits into from
Feb 16, 2023

Conversation

hecksmosis
Copy link
Contributor

Resolved / Related Issues
Items resolved / related issues by this PR.

Validation
How did you test these changes?

  • Built and ran the app

@yaira2
Copy link
Member

yaira2 commented Feb 14, 2023

Is this working for you? I don't see the option and it's crashing as well.

@hecksmosis
Copy link
Contributor Author

This works fine for me

@hecksmosis
Copy link
Contributor Author

One of the few times CI doesn't spontaneously combust

@yaira2
Copy link
Member

yaira2 commented Feb 14, 2023

It's still not working for me, maybe someone else can test

@yaira2
Copy link
Member

yaira2 commented Feb 15, 2023

image

@hecksmosis
Copy link
Contributor Author

I cannot reproduce this, don't know why this is happening

@hecksmosis
Copy link
Contributor Author

Does this happen in main?

@yaira2
Copy link
Member

yaira2 commented Feb 15, 2023

main is fine

@yaira2
Copy link
Member

yaira2 commented Feb 15, 2023

@hishitetsu does this work for you?

@yaira2
Copy link
Member

yaira2 commented Feb 15, 2023

@hecksmosis in the meantime, can you share a screen recording of this in action?

@hishitetsu
Copy link
Member

hishitetsu commented Feb 15, 2023

Sorry, I'm away from home and can't debug right now, but showOpenWithMenu: true was not specified in the other widgets and was causing the shell extension not to load, so I removed this option in #11270 .

@hecksmosis
Copy link
Contributor Author

@hishitetsu I looked into this and it seems to work but only if you don't await it

@hecksmosis
Copy link
Contributor Author

I'll send a screen recording

@hecksmosis
Copy link
Contributor Author

f1fcd40d-7694-4ae8-b9c3-5314700cc655.webm

@hishitetsu
Copy link
Member

hishitetsu commented Feb 16, 2023

I have identified the cause of the error.
Once #11325 is merged, the error should no longer occur.

The cause of the error was not whether there was await
The process flow here would be much the same with or without await, I think.

@yaira2
Copy link
Member

yaira2 commented Feb 16, 2023

It's working now but it looks like it's missing the placeholder that's supposed to show when the sub menu is loading. Also, there seems to be an issue in menu flyouts where the colored icons don't work so we might have to use a regular icon for now.

@hecksmosis
Copy link
Contributor Author

It's working now but it looks like it's missing the placeholder that's supposed to show when the sub menu is loading. Also, there seems to be an issue in menu flyouts where the colored icons don't work so we might have to use a regular icon for now.

@yaira2 Do you mean a placeholder for the open with menu so it doesn't change size?

@yaira2
Copy link
Member

yaira2 commented Feb 16, 2023

Yes

@hecksmosis
Copy link
Contributor Author

Ok, on it

@yaira2
Copy link
Member

yaira2 commented Feb 16, 2023

\uE17D

@hecksmosis
Copy link
Contributor Author

Done

Copy link
Member

@yaira2 yaira2 left a comment

Choose a reason for hiding this comment

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

LGTM

@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Feb 16, 2023
@yaira2 yaira2 merged commit 6922c25 into files-community:main Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Move "open with" to main menu when right clicking recent files
3 participants