New issue
Advanced search Search tips

Issue 887135 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

NetworkFileShares UI: Create freeform Add Share dialog

Project Member Reported by baileyberro@chromium.org, Sep 19

Issue description

Pull the Add Share dialog out of settings so that it can be surfaced from the Files App without having to open the Settings App.
 
Components: Platform>Apps>FileManager
Probably won't make 71.
Labels: OS-Chrome
Labels: -M-71 M-72
Labels: -Pri-2 Pri-1
Owner: baileyberro@chromium.org
Status: Assigned (was: Available)
Status: Started (was: Assigned)
Labels: CrOSFilesFeature-SMB
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 30

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ac58aedbab021195cc5ff737abbea62eba7ed6b1

commit ac58aedbab021195cc5ff737abbea62eba7ed6b1
Author: Bailey Berro <baileyberro@chromium.org>
Date: Tue Oct 30 03:06:49 2018

Migrate add_smb_share dialog to a cr_component

This is the first in a series of changes to create a freeform dialog
for adding an SMB share in addition to the dialog existing in settings.

- Moves add_smb_share dialog to cr_components
- Switches $i18n{} to i18n()
- Updates BUILD files

Bug: chromium:887135
Test: verified on test device
Change-Id: I1133d9e45ee7ac63e338e22e4e69e3e000a3ee02
Reviewed-on: https://chromium-review.googlesource.com/c/1281868
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603763}
[modify] https://crrev.com/ac58aedbab021195cc5ff737abbea62eba7ed6b1/chrome/browser/resources/settings/downloads_page/BUILD.gn
[modify] https://crrev.com/ac58aedbab021195cc5ff737abbea62eba7ed6b1/chrome/browser/resources/settings/downloads_page/smb_shares_page.html
[modify] https://crrev.com/ac58aedbab021195cc5ff737abbea62eba7ed6b1/chrome/browser/resources/settings/settings_resources.grd
[modify] https://crrev.com/ac58aedbab021195cc5ff737abbea62eba7ed6b1/chrome/test/data/webui/settings/smb_shares_page_tests.js
[rename] https://crrev.com/ac58aedbab021195cc5ff737abbea62eba7ed6b1/ui/webui/resources/cr_components/.eslintrc.js
[add] https://crrev.com/ac58aedbab021195cc5ff737abbea62eba7ed6b1/ui/webui/resources/cr_components/chromeos/smb_shares/BUILD.gn
[add] https://crrev.com/ac58aedbab021195cc5ff737abbea62eba7ed6b1/ui/webui/resources/cr_components/chromeos/smb_shares/OWNERS
[rename] https://crrev.com/ac58aedbab021195cc5ff737abbea62eba7ed6b1/ui/webui/resources/cr_components/chromeos/smb_shares/add_smb_share_dialog.html
[rename] https://crrev.com/ac58aedbab021195cc5ff737abbea62eba7ed6b1/ui/webui/resources/cr_components/chromeos/smb_shares/add_smb_share_dialog.js
[rename] https://crrev.com/ac58aedbab021195cc5ff737abbea62eba7ed6b1/ui/webui/resources/cr_components/chromeos/smb_shares/smb_browser_proxy.html
[rename] https://crrev.com/ac58aedbab021195cc5ff737abbea62eba7ed6b1/ui/webui/resources/cr_components/chromeos/smb_shares/smb_browser_proxy.js
[modify] https://crrev.com/ac58aedbab021195cc5ff737abbea62eba7ed6b1/ui/webui/resources/cr_components/cr_components_resources.grdp

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 30

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/04d3d76184bb06e84d58a59a084c41f9aaaef155

commit 04d3d76184bb06e84d58a59a084c41f9aaaef155
Author: Bailey Berro <baileyberro@chromium.org>
Date: Tue Oct 30 21:12:06 2018

Move SmbHandler out of settings

This change is a part of an ongoing series of changes to pull the add
SMB share dialog out of settings.

- Moves SmbHandler from ui/webui/settings/chromeos to ui/webui/chromeos
- Changes SmbHandler from implementing a settings-specific
WebUIMessageHandler to the more general version
- Removes unused overridden methods

Bug: chromium:887135
Change-Id: If196d3bdf53ec69d46ea25f1314a0bafff6c296d
Reviewed-on: https://chromium-review.googlesource.com/c/1285519
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604013}
[modify] https://crrev.com/04d3d76184bb06e84d58a59a084c41f9aaaef155/chrome/browser/ui/BUILD.gn
[add] https://crrev.com/04d3d76184bb06e84d58a59a084c41f9aaaef155/chrome/browser/ui/webui/chromeos/smb_shares/OWNERS
[rename] https://crrev.com/04d3d76184bb06e84d58a59a084c41f9aaaef155/chrome/browser/ui/webui/chromeos/smb_shares/smb_handler.cc
[rename] https://crrev.com/04d3d76184bb06e84d58a59a084c41f9aaaef155/chrome/browser/ui/webui/chromeos/smb_shares/smb_handler.h
[modify] https://crrev.com/04d3d76184bb06e84d58a59a084c41f9aaaef155/chrome/browser/ui/webui/settings/md_settings_ui.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/225c8800dd363b23cbf6c7cd9a90361536a601b3

commit 225c8800dd363b23cbf6c7cd9a90361536a601b3
Author: Bailey Berro <baileyberro@chromium.org>
Date: Wed Oct 31 01:37:18 2018

Add smb_dialog::AddLocalizedStrings

This change is part of an ongoing series of changes to pull the
add-smb-share dialog out of settings.

- Creates smb_shares_localized_strings_provider
- Switches the settings localized strings provider to using the
smb_shares_localized_strings_provider

Bug: chromium:887135
Change-Id: If061c2175497188772cfaa06cccb4727e438e962
Reviewed-on: https://chromium-review.googlesource.com/c/1287037
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604100}
[modify] https://crrev.com/225c8800dd363b23cbf6c7cd9a90361536a601b3/chrome/browser/ui/BUILD.gn
[add] https://crrev.com/225c8800dd363b23cbf6c7cd9a90361536a601b3/chrome/browser/ui/webui/chromeos/smb_shares/smb_shares_localized_strings_provider.cc
[add] https://crrev.com/225c8800dd363b23cbf6c7cd9a90361536a601b3/chrome/browser/ui/webui/chromeos/smb_shares/smb_shares_localized_strings_provider.h
[modify] https://crrev.com/225c8800dd363b23cbf6c7cd9a90361536a601b3/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Nov 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0c7bb2bfdbc47d2f856e307bdd94dd2f5b63c906

commit 0c7bb2bfdbc47d2f856e307bdd94dd2f5b63c906
Author: Bailey Berro <baileyberro@chromium.org>
Date: Fri Nov 09 18:47:36 2018

Clean up SmbShare  build guards for Chrome OS

Adds is_chromeos checks to the build files for the add-smb-share-dialog

Bug: chromium:887135
Change-Id: I7ba729324c9e523f82924927f6eab86be8e4247f
Reviewed-on: https://chromium-review.googlesource.com/c/1323806
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606916}
[modify] https://crrev.com/0c7bb2bfdbc47d2f856e307bdd94dd2f5b63c906/chrome/browser/resources/settings/downloads_page/BUILD.gn
[modify] https://crrev.com/0c7bb2bfdbc47d2f856e307bdd94dd2f5b63c906/ui/webui/resources/cr_components/chromeos/smb_shares/BUILD.gn

Project Member

Comment 12 by bugdroid1@chromium.org, Nov 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5a991af44134547b2395123c97afa5a7ac8ea90f

commit 5a991af44134547b2395123c97afa5a7ac8ea90f
Author: Bailey Berro <baileyberro@chromium.org>
Date: Fri Nov 09 22:41:01 2018

Create free form Add SMB Share dialog

- Creates a free form SMB share dialog
- Dialog will be opened by SmbProvider::RequestMount

Bug: chromium:887135
Change-Id: I379417ac15d633bce704bd50c8d31e8488fc2975
Reviewed-on: https://chromium-review.googlesource.com/c/1289409
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607008}
[modify] https://crrev.com/5a991af44134547b2395123c97afa5a7ac8ea90f/chrome/browser/browser_resources.grd
[modify] https://crrev.com/5a991af44134547b2395123c97afa5a7ac8ea90f/chrome/browser/resources/chromeos/BUILD.gn
[add] https://crrev.com/5a991af44134547b2395123c97afa5a7ac8ea90f/chrome/browser/resources/chromeos/smb_shares/BUILD.gn
[add] https://crrev.com/5a991af44134547b2395123c97afa5a7ac8ea90f/chrome/browser/resources/chromeos/smb_shares/OWNERS
[add] https://crrev.com/5a991af44134547b2395123c97afa5a7ac8ea90f/chrome/browser/resources/chromeos/smb_shares/smb_share_dialog.html
[add] https://crrev.com/5a991af44134547b2395123c97afa5a7ac8ea90f/chrome/browser/resources/chromeos/smb_shares/smb_share_dialog.js
[add] https://crrev.com/5a991af44134547b2395123c97afa5a7ac8ea90f/chrome/browser/resources/chromeos/smb_shares/smb_share_dialog_container.html
[modify] https://crrev.com/5a991af44134547b2395123c97afa5a7ac8ea90f/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/5a991af44134547b2395123c97afa5a7ac8ea90f/chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc
[add] https://crrev.com/5a991af44134547b2395123c97afa5a7ac8ea90f/chrome/browser/ui/webui/chromeos/smb_shares/smb_share_dialog.cc
[add] https://crrev.com/5a991af44134547b2395123c97afa5a7ac8ea90f/chrome/browser/ui/webui/chromeos/smb_shares/smb_share_dialog.h
[modify] https://crrev.com/5a991af44134547b2395123c97afa5a7ac8ea90f/chrome/common/webui_url_constants.cc
[modify] https://crrev.com/5a991af44134547b2395123c97afa5a7ac8ea90f/chrome/common/webui_url_constants.h
[modify] https://crrev.com/5a991af44134547b2395123c97afa5a7ac8ea90f/ui/webui/resources/cr_components/chromeos/smb_shares/BUILD.gn

Project Member

Comment 13 by bugdroid1@chromium.org, Nov 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9c9c1d2614afbbf2eff1ed246ef27517a39b4bf1

commit 9c9c1d2614afbbf2eff1ed246ef27517a39b4bf1
Author: Bailey Berro <baileyberro@chromium.org>
Date: Tue Nov 27 21:30:06 2018

Move SMB share error toast to dialog

This change moves the error toast to the dialog when mounting an SMB
share.

Previously the flow was:

- Click add mount
- Dialog closes to SMB Shares parent page
- Error/success string is displayed on parent page

Now the flow will be:
- Click add mount
- Dialog stays open but add button becomes disabled
- Error/success string is displayed on dialog itself

An added advantage of this change is that it improves user experience
as it is easier for users to debug error cases when the dialog is still
open and they can still see the information they entered.

Bug: chromium:887135
Test: browser_tests --gtest_filter=CrSettingsSmb*
Change-Id: I6f68cd71a0002372a45975b3d8602809b36a6754
Reviewed-on: https://chromium-review.googlesource.com/c/1316664
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Reviewed-by: Scott Chen <scottchen@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611336}
[modify] https://crrev.com/9c9c1d2614afbbf2eff1ed246ef27517a39b4bf1/chrome/browser/resources/settings/downloads_page/smb_shares_page.html
[modify] https://crrev.com/9c9c1d2614afbbf2eff1ed246ef27517a39b4bf1/chrome/browser/resources/settings/downloads_page/smb_shares_page.js
[modify] https://crrev.com/9c9c1d2614afbbf2eff1ed246ef27517a39b4bf1/chrome/browser/ui/webui/chromeos/smb_shares/smb_handler.cc
[modify] https://crrev.com/9c9c1d2614afbbf2eff1ed246ef27517a39b4bf1/chrome/browser/ui/webui/chromeos/smb_shares/smb_handler.h
[modify] https://crrev.com/9c9c1d2614afbbf2eff1ed246ef27517a39b4bf1/chrome/browser/ui/webui/chromeos/smb_shares/smb_shares_localized_strings_provider.cc
[modify] https://crrev.com/9c9c1d2614afbbf2eff1ed246ef27517a39b4bf1/chrome/test/data/webui/settings/smb_shares_page_tests.js
[modify] https://crrev.com/9c9c1d2614afbbf2eff1ed246ef27517a39b4bf1/ui/webui/resources/cr_components/chromeos/smb_shares/add_smb_share_dialog.html
[modify] https://crrev.com/9c9c1d2614afbbf2eff1ed246ef27517a39b4bf1/ui/webui/resources/cr_components/chromeos/smb_shares/add_smb_share_dialog.js
[modify] https://crrev.com/9c9c1d2614afbbf2eff1ed246ef27517a39b4bf1/ui/webui/resources/cr_components/chromeos/smb_shares/smb_browser_proxy.js

Project Member

Comment 14 by bugdroid1@chromium.org, Dec 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4b4e71d130ec3831809f04f8c03e743def1fc29f

commit 4b4e71d130ec3831809f04f8c03e743def1fc29f
Author: Bailey Berro <baileyberro@chromium.org>
Date: Mon Dec 03 19:41:13 2018

Add ShouldOpenFileManagerAfterMount attribute to add-smb-share-dialog

This change adds an InSettings attribute that is used to determine whether
the File Manager should be automatically opened on a successful mount call.
When mounting from the free-form dialog, the File Manager will not be
automatically opened.

Bug: chromium:887135
Test: browser_tests --gtest_filter=CrSettingsSmb*
Change-Id: I4e681c1ffe104f234784c2f958d24b2d257bc33c
Reviewed-on: https://chromium-review.googlesource.com/c/1336911
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Scott Chen <scottchen@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613197}
[modify] https://crrev.com/4b4e71d130ec3831809f04f8c03e743def1fc29f/chrome/browser/chromeos/smb_client/smb_service.cc
[modify] https://crrev.com/4b4e71d130ec3831809f04f8c03e743def1fc29f/chrome/browser/chromeos/smb_client/smb_service.h
[modify] https://crrev.com/4b4e71d130ec3831809f04f8c03e743def1fc29f/chrome/browser/resources/settings/downloads_page/smb_shares_page.html
[modify] https://crrev.com/4b4e71d130ec3831809f04f8c03e743def1fc29f/chrome/browser/ui/webui/chromeos/smb_shares/smb_handler.cc
[modify] https://crrev.com/4b4e71d130ec3831809f04f8c03e743def1fc29f/chrome/test/data/webui/settings/smb_shares_page_tests.js
[modify] https://crrev.com/4b4e71d130ec3831809f04f8c03e743def1fc29f/ui/webui/resources/cr_components/chromeos/smb_shares/add_smb_share_dialog.js
[modify] https://crrev.com/4b4e71d130ec3831809f04f8c03e743def1fc29f/ui/webui/resources/cr_components/chromeos/smb_shares/smb_browser_proxy.js

Project Member

Comment 15 by bugdroid1@chromium.org, Dec 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/242c217f39517d207019fa929fe41566660539a2

commit 242c217f39517d207019fa929fe41566660539a2
Author: Bailey Berro <baileyberro@chromium.org>
Date: Mon Dec 03 21:58:12 2018

Remove call unneeded Attached() method call for Smb Dialog

This change removes the attached method override from the smb-share-dialog
component.

Bug: chromium:887135
Change-Id: Id6a7b06a2b2b9ea4d0da5dc04cb14f7143c0c312
Reviewed-on: https://chromium-review.googlesource.com/c/1340860
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613271}
[modify] https://crrev.com/242c217f39517d207019fa929fe41566660539a2/chrome/browser/resources/chromeos/smb_shares/smb_share_dialog.js

Project Member

Comment 16 by bugdroid1@chromium.org, Dec 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c83aeb22ecd0b316c686b604be9913192ac86591

commit c83aeb22ecd0b316c686b604be9913192ac86591
Author: Bailey Berro <baileyberro@chromium.org>
Date: Tue Dec 04 19:03:29 2018

Switch to free-form dialog when adding an SMB share from File Manager.

This change flips the bit to switch from popping the SMB Shares settings
page to opening the freeform dialog on top of File Manager when adding an
SMB Share from the Add New Services menu.

Bug: chromium:887135
Change-Id: Idb54f28373abb79a722408c6ba8f21155c3f6d3b
Reviewed-on: https://chromium-review.googlesource.com/c/1340792
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613622}
[modify] https://crrev.com/c83aeb22ecd0b316c686b604be9913192ac86591/chrome/browser/chromeos/smb_client/smb_provider.cc

Is this fixed in M-72 or should we bump to M-73?
Project Member

Comment 18 by bugdroid1@chromium.org, Jan 16 (6 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fd865f48a23910d65be86fe79b4366f3f0e63226

commit fd865f48a23910d65be86fe79b4366f3f0e63226
Author: Bailey Berro <baileyberro@chromium.org>
Date: Wed Jan 16 22:48:46 2019

Add support for an error message to cr-searchable-drop-down

- This change updates cr-searchable-drop-down to support setting an 'error-message'
property. If 'error-message' is set, the input field is marked as invalid and the
error message is displayed beneath.
- Consumers of cr-searchable-drop-down can set a new
'error-message-allowed' attribute to increase the vertical space of the
cr-searchable-drop-down to allow space for this error message

Bug: chromium:887135
Change-Id: I8a81132a00d9c519222027503434f6f1c417608d
Reviewed-on: https://chromium-review.googlesource.com/c/1381833
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623416}
[modify] https://crrev.com/fd865f48a23910d65be86fe79b4366f3f0e63226/chrome/test/data/webui/cr_elements/cr_searchable_drop_down_tests.js
[modify] https://crrev.com/fd865f48a23910d65be86fe79b4366f3f0e63226/ui/webui/resources/cr_elements/cr_searchable_drop_down/cr_searchable_drop_down.html
[modify] https://crrev.com/fd865f48a23910d65be86fe79b4366f3f0e63226/ui/webui/resources/cr_elements/cr_searchable_drop_down/cr_searchable_drop_down.js

Project Member

Comment 19 by bugdroid1@chromium.org, Jan 17 (5 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0402e0519a20ae80fb4e5e98e39643351f86ea39

commit 0402e0519a20ae80fb4e5e98e39643351f86ea39
Author: Bailey Berro <baileyberro@chromium.org>
Date: Thu Jan 17 21:52:12 2019

Remove error toast and replace with inline error messages.

This change removes the error toast that was previously used to display
the result when mounting a share. Instead, errors will be shown next to
the relevant fields.

Bug: chromium:887135
Change-Id: I9ca2cad98be7f6375b0576f7eeee5d2ef58bfc48
Reviewed-on: https://chromium-review.googlesource.com/c/1383408
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623864}
[modify] https://crrev.com/0402e0519a20ae80fb4e5e98e39643351f86ea39/chrome/browser/ui/webui/chromeos/smb_shares/smb_share_dialog.cc
[modify] https://crrev.com/0402e0519a20ae80fb4e5e98e39643351f86ea39/ui/webui/resources/cr_components/chromeos/smb_shares/add_smb_share_dialog.html
[modify] https://crrev.com/0402e0519a20ae80fb4e5e98e39643351f86ea39/ui/webui/resources/cr_components/chromeos/smb_shares/add_smb_share_dialog.js

Sign in to add a comment