-
Notifications
You must be signed in to change notification settings - Fork 114
Remove electron file_dialog implementation #385
Conversation
requires brave/muon#385 Auditors: @bridiver, @bbondy, @bsclifton Test Plan:
requires brave/muon#385 Auditors: @bridiver, @bbondy, @bsclifton Test Plan:
Tested on all platforms. Ready for review. |
requires brave/muon#385 Auditors: @bridiver, @bbondy, @bsclifton Test Plan: Specified in PR
settings.default_path.Append(base::FilePath::FromUTF8Unsafe(url)); | ||
settings.type = ui::SelectFileDialog::SELECT_SAVEAS_FILE; | ||
std::vector<base::FilePath::StringType> extensions; | ||
extensions.push_back( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why json?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this should be specific to the performance profile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will remove it.
atom/browser/ui/file_dialog.cc
Outdated
@@ -0,0 +1,163 @@ | |||
// Copyright (c) 2017 Brave Authors. All rights reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we use file_select_helper instead or would that be a lot more work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it will requires more extra work including whole API interfaces changes. Can we do it as follow-up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the api for browser-laptop or just internal muon?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is both, they all use the same file_dialog interfaces now
working on using |
requires brave/muon#385 Auditors: @bridiver, @bbondy, @bsclifton Test Plan: Specified in PR
DONE! Please review it again @bridiver |
fix #242 Auditors: @bridiver, @bbondy, @bsclifton
merged as part of C64 |
requires brave/muon#385 Auditors: @bridiver, @bbondy, @bsclifton Test Plan: Specified in PR
requires brave/muon#385 Auditors: @bridiver, @bbondy, @bsclifton Test Plan: Specified in PR
fix #242
requires brave/browser-laptop#11906
Auditors: @bridiver, @bbondy, @bsclifton