Vertical tab - drag&drop feature for link and text. #19785

thirumurugan-git · 2023-08-20T23:28:15Z

Adds drag & drop(link and text) feature to vertical tabs.

vtab-drag-n-drop.webm

Resolves brave/brave-browser#31492

Submitter Checklist:

I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
There is a ticket for my issue
Used Github auto-closing keywords in the PR description above
Wrote a good PR/commit description
Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
Checked the PR locally:
- npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
- npm run lint, npm run presubmit wiki, npm run gn_check, npm run tslint
Ran git rebase master (if needed)

Reviewer Checklist:

A security review is not needed, or a link to one is included in the PR description
New files have MPL-2.0 license header
Adequate test coverage exists to prevent regressions
Major classes, functions and non-trivial code blocks are well-commented
Changes in component dependencies are properly reflected in gn
Code follows the style guide
Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

thirumurugan-git · 2023-08-20T23:29:41Z

@sangwoo108 Kindly review this PR.

sangwoo108

Thanks for the PR! Impressive work!

browser/ui/views/frame/vertical_tab_strip_root_view.h

browser/ui/views/frame/vertical_tab_strip_root_view.cc

browser/ui/views/frame/vertical_tab_strip_root_view_browsertest.cc

browser/ui/views/tabs/brave_compound_tab_container.cc

browser/ui/views/tabs/brave_tab_container.cc

browser/ui/views/frame/vertical_tab_strip_root_view.h

sangwoo108 · 2023-08-21T01:07:10Z

Updated issue description to link the issue that this PR fixes

thirumurugan-git · 2023-08-22T05:34:00Z

@sangwoo108 I've implemented the required changes. Could you please verify them?

browser/ui/views/tabs/brave_compound_tab_container.cc

browser/ui/views/tabs/brave_tab_container.cc

sangwoo108 · 2023-08-22T07:38:36Z

@simonhong Could you double check this PR?

simonhong

👍🏼

browser/ui/views/tabs/brave_tab_container.cc

browser/ui/views/frame/vertical_tab_strip_root_view.cc

browser/ui/views/frame/vertical_tab_strip_root_view.h

browser/ui/views/frame/vertical_tab_strip_root_view_browsertest.cc

browser/ui/views/frame/vertical_tab_strip_root_view.h

thirumurugan-git · 2023-09-02T19:31:44Z

@sangwoo108 I've used the Edge as a reference for dropping links or text onto pinned tabs. In Edge, hot_width is calculated for pinned tabs rather than hot_height. Is there another approach or strategy we should consider for this?

sangwoo108 · 2023-09-03T10:44:37Z

In Edge, hot_width is calculated for pinned tabs rather than hot_height

I think hot_height is more proper for us. Did you find using hot_width is better?

thirumurugan-git · 2023-09-04T09:45:48Z

When the tab is expanded, and pinned tabs are displayed in a grid view, we need to show a downward indicator for pinned tabs. In this case, we must also take the x-axis into consideration.

sangwoo108 · 2023-09-04T12:55:34Z

and pinned tabs are displayed in a grid view, we need to show a downward indicator for pinned tabs. In this case, we must also take the x-axis into consideration.

Well, when we drop a link in between pinned tabs, a new tab seems to be created as normal tab. So I feel like we don't have to consider {hot_width/height} of pinned tabs at all. But I agree that the arrow is downward as you said, horizontal axis would be better.

thirumurugan-git · 2023-09-04T13:48:48Z

@sangwoo108 Considering the 'hot_width' of the tab, I've calculated the drop index, which is then used to determine the drop bounds of the indicator. Are you suggesting that I should set this 'hot_width' to 0 so that the entire width of the pinned tab is taken into account for dropping?

sangwoo108 · 2023-09-05T01:00:26Z

@sangwoo108 Considering the 'hot_width' of the tab, I've calculated the drop index, which is then used to determine the drop bounds of the indicator. Are you suggesting that I should set this 'hot_width' to 0 so that the entire width of the pinned tab is taken into account for dropping?

I think you can leave it as you implemeneted. We might want to create a pinned tabs when it's in the hot width. But it should be discussed with other folks.

browser/ui/views/frame/vertical_tab_strip_root_view.cc

app/theme/brave_theme_resources.grd

browser/ui/views/tabs/brave_tab_container.cc

sangwoo108 · 2023-09-05T04:55:20Z

Thanks for your effort on this @thirumurugan-git

sangwoo108

LGTM with a few nits 👍 Let me run our CI again.

browser/ui/views/frame/vertical_tab_strip_root_view.cc

browser/ui/views/frame/vertical_tab_strip_root_view.h

browser/ui/views/frame/vertical_tab_strip_root_view.cc

browser/ui/views/tabs/brave_tab_container.cc

thirumurugan-git · 2023-09-10T14:20:19Z

@sangwoo108 I have made the changes. Could you please review them?

sangwoo108 · 2023-09-11T00:29:45Z

I see some errors from npm run lint. Let me rebase and fix them.

sangwoo108 · 2023-09-11T02:20:02Z

Hi, @thirumurugan-git , Our CI reports that some tests fails. It might be a platform specific things but I'm hoping you could check out. This happens on a Windows bot.

[2023-09-11T02:13:31.732Z] 1 test failed:

[2023-09-11T02:13:31.732Z]     VerticalTabStripRootViewBrowserTest.DragOnCurrentTab (..\..\brave\browser\ui\views\frame\vertical_tab_strip_root_view_browsertest.cc:112)

[2023-09-11T02:13:31.732Z] 1 test crashed:

[2023-09-11T02:13:31.732Z]     VerticalTabStripRootViewBrowserTest.DragAfterCurrentTab (..\..\brave\browser\ui\views\frame\vertical_tab_strip_root_view_browsertest.cc:73)

thirumurugan-git · 2023-09-11T08:15:56Z

@sangwoo108 I am unable to debug the issue on my machine as I do not have a Windows machine. Could you please share the stack trace so that I can fix it on my branch?

sangwoo108 · 2023-09-11T11:44:52Z

Sure, here you are. If you think this failures are Windows specific thing due to some limitation of test env, you can disable the tests on Windows with following pattern

stacks

#if BUILDFLAG(IS_WIN)
#define MAYBE_MyTest DISABLED_MyTest
#else
#define MAYBE_MyTest MyTest
#endif

IN_PROC_BROWSER_TEST_F(FooTest, MAYBE_MyTest) {

[ RUN      ] VerticalTabStripRootViewBrowserTest.DragAfterCurrentTab

[5312:7940:0911/031918.126:WARNING:chrome_main_delegate.cc(609)] This is Chrome version 117.1.60.2 (not a warning)

[5312:7940:0911/031918.129:WARNING:policy_logger.cc(148)] :components\enterprise\browser\controller\chrome_browser_cloud_management_controller.cc(88) Could not create policy manager as CBCM is not enabled.

[5312:7940:0911/031918.158:ERROR:policy_logger.cc(154)] :components\enterprise\browser\controller\chrome_browser_cloud_management_controller.cc(163) Cloud management controller initialization aborted as CBCM is not enabled.

[5312:7940:0911/031918.180:WARNING:external_provider_impl.cc(513)] Malformed extension dictionary for extension: odbfpeeihdkbihmopkbjmoonfanlbfcl.  Key external_update_url has value "", which is not a valid URL.

..\..\brave\browser\ui\views\frame\vertical_tab_strip_root_view_browsertest.cc(103): error: Expected equality of these values:

  output_drag_op

    Which is: kNone

  ui::mojom::DragOperation::kCopy

    Which is: kCopy

Stack trace:

	VerticalTabStripRootViewBrowserTest_DragAfterCurrentTab_Test::RunTestOnMainThread [0x00007FF70752C3A5+1541] (C:\ws\src\brave\browser\ui\views\frame\vertical_tab_strip_root_view_browsertest.cc:103)

	content::BrowserTestBase::ProxyRunTestOnMainThreadLoop [0x00007FF70CAB8E29+1001] (C:\ws\src\content\public\test\browser_test_base.cc:913)

	content::BrowserMainLoop::InterceptMainMessageLoopRun [0x00007FF709AF1BB2+112] (C:\ws\src\content\browser\browser_main_loop.cc:1042)

	content::BrowserMainLoop::RunMainMessageLoop [0x00007FF709AF1BFF+37] (C:\ws\src\content\browser\browser_main_loop.cc:1054)

	content::BrowserMainRunnerImpl::Run [0x00007FF709AF3B51+17] (C:\ws\src\content\browser\browser_main_runner_impl.cc:160)

	content::BrowserMain [0x00007FF709AEED0E+166] (C:\ws\src\content\browser\browser_main.cc:34)

	content::RunBrowserProcessMain [0x00007FF70AD8C05A+254] (C:\ws\src\content\app\content_main_runner_impl.cc:685)

	content::ContentMainRunnerImpl::RunBrowser [0x00007FF70AD8D1FC+408] (C:\ws\src\content\app\content_main_runner_impl.cc:1266)

	content::ContentMainRunnerImpl::Run [0x00007FF70AD8D02A+890] (C:\ws\src\content\app\content_main_runner_impl.cc:1120)

	content::RunContentProcess [0x00007FF70AD8B7A5+886] (C:\ws\src\content\app\content_main.cc:326)

	content::ContentMain [0x00007FF70AD8BA40+113] (C:\ws\src\content\app\content_main.cc:343)

	content::BrowserTestBase::SetUp [0x00007FF70CAB82F5+2437] (C:\ws\src\content\public\test\browser_test_base.cc:587)

	InProcessBrowserTest::SetUp [0x00007FF70B658E64+404] (C:\ws\src\chrome\test\base\in_process_browser_test.cc:503)



..\..\brave\browser\ui\views\frame\vertical_tab_strip_root_view_browsertest.cc(104): error: Expected equality of these values:

  tab_strip_model->count()

    Which is: 1

  2

Stack trace:

	VerticalTabStripRootViewBrowserTest_DragAfterCurrentTab_Test::RunTestOnMainThread [0x00007FF70752C498+1784] (C:\ws\src\brave\browser\ui\views\frame\vertical_tab_strip_root_view_browsertest.cc:104)

	content::BrowserTestBase::ProxyRunTestOnMainThreadLoop [0x00007FF70CAB8E29+1001] (C:\ws\src\content\public\test\browser_test_base.cc:913)

	content::BrowserMainLoop::InterceptMainMessageLoopRun [0x00007FF709AF1BB2+112] (C:\ws\src\content\browser\browser_main_loop.cc:1042)

	content::BrowserMainLoop::RunMainMessageLoop [0x00007FF709AF1BFF+37] (C:\ws\src\content\browser\browser_main_loop.cc:1054)

	content::BrowserMainRunnerImpl::Run [0x00007FF709AF3B51+17] (C:\ws\src\content\browser\browser_main_runner_impl.cc:160)

	content::BrowserMain [0x00007FF709AEED0E+166] (C:\ws\src\content\browser\browser_main.cc:34)

	content::RunBrowserProcessMain [0x00007FF70AD8C05A+254] (C:\ws\src\content\app\content_main_runner_impl.cc:685)

	content::ContentMainRunnerImpl::RunBrowser [0x00007FF70AD8D1FC+408] (C:\ws\src\content\app\content_main_runner_impl.cc:1266)

	content::ContentMainRunnerImpl::Run [0x00007FF70AD8D02A+890] (C:\ws\src\content\app\content_main_runner_impl.cc:1120)

	content::RunContentProcess [0x00007FF70AD8B7A5+886] (C:\ws\src\content\app\content_main.cc:326)

	content::ContentMain [0x00007FF70AD8BA40+113] (C:\ws\src\content\app\content_main.cc:343)

	content::BrowserTestBase::SetUp [0x00007FF70CAB82F5+2437] (C:\ws\src\content\public\test\browser_test_base.cc:587)

	InProcessBrowserTest::SetUp [0x00007FF70B658E64+404] (C:\ws\src\chrome\test\base\in_process_browser_test.cc:503)



Received fatal exception EXCEPTION_ACCESS_VIOLATION

	VerticalTabStripRootViewBrowserTest_DragAfterCurrentTab_Test::RunTestOnMainThread [0x00007FF70752C50B+1899] (C:\ws\src\brave\browser\ui\views\frame\vertical_tab_strip_root_view_browsertest.cc:105)

	content::BrowserTestBase::ProxyRunTestOnMainThreadLoop [0x00007FF70CAB8E29+1001] (C:\ws\src\content\public\test\browser_test_base.cc:913)

	content::BrowserMainLoop::InterceptMainMessageLoopRun [0x00007FF709AF1BB2+112] (C:\ws\src\content\browser\browser_main_loop.cc:1042)

	content::BrowserMainLoop::RunMainMessageLoop [0x00007FF709AF1BFF+37] (C:\ws\src\content\browser\browser_main_loop.cc:1054)

	content::BrowserMainRunnerImpl::Run [0x00007FF709AF3B51+17] (C:\ws\src\content\browser\browser_main_runner_impl.cc:160)

	content::BrowserMain [0x00007FF709AEED0E+166] (C:\ws\src\content\browser\browser_main.cc:34)

	content::RunBrowserProcessMain [0x00007FF70AD8C05A+254] (C:\ws\src\content\app\content_main_runner_impl.cc:685)

	content::ContentMainRunnerImpl::RunBrowser [0x00007FF70AD8D1FC+408] (C:\ws\src\content\app\content_main_runner_impl.cc:1266)

	content::ContentMainRunnerImpl::Run [0x00007FF70AD8D02A+890] (C:\ws\src\content\app\content_main_runner_impl.cc:1120)

	content::RunContentProcess [0x00007FF70AD8B7A5+886] (C:\ws\src\content\app\content_main.cc:326)

	content::ContentMain [0x00007FF70AD8BA40+113] (C:\ws\src\content\app\content_main.cc:343)

	content::BrowserTestBase::SetUp [0x00007FF70CAB82F5+2437] (C:\ws\src\content\public\test\browser_test_base.cc:587)

	InProcessBrowserTest::SetUp [0x00007FF70B658E64+404] (C:\ws\src\chrome\test\base\in_process_browser_test.cc:503)

	testing::Test::Run [0x00007FF707A291E8+136] (C:\ws\src\third_party\googletest\src\googletest\src\gtest.cc:2668)

	testing::TestInfo::Run [0x00007FF707A29C52+262] (C:\ws\src\third_party\googletest\src\googletest\src\gtest.cc:2852)

	testing::TestSuite::Run [0x00007FF707A2A4A1+361] (C:\ws\src\third_party\googletest\src\googletest\src\gtest.cc:3010)

	testing::internal::UnitTestImpl::RunAllTests [0x00007FF707A34376+710] (C:\ws\src\third_party\googletest\src\googletest\src\gtest.cc:5867)

	testing::UnitTest::Run [0x00007FF707A34023+209] (C:\ws\src\third_party\googletest\src\googletest\src\gtest.cc:5440)

	base::TestSuite::Run [0x00007FF70B7BC706+166] (C:\ws\src\base\test\test_suite.cc:406)

	ChromeTestSuiteRunner::RunTestSuiteInternal [0x00007FF711CCD5CD+53] (C:\ws\src\chrome\test\base\chrome_test_launcher.cc:91)

	ChromeTestSuiteRunner::RunTestSuite [0x00007FF711CCD64C+92] (C:\ws\src\chrome\test\base\chrome_test_launcher.cc:96)

	content::LaunchTestsInternal [0x00007FF70E6673D1+749] (C:\ws\src\content\public\test\test_launcher.cc:405)

	content::LaunchTests [0x00007FF70E6677CF+199] (C:\ws\src\content\public\test\test_launcher.cc:479)

	LaunchChromeTests [0x00007FF711CCDC22+266] (C:\ws\src\chrome\test\base\chrome_test_launcher.cc:333)

	main [0x00007FF711CCD4BA+166] (C:\ws\src\brave\test\base\browser_tests_main.cc:36)

	__scrt_common_main_seh [0x00007FF712049188+268] (D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)

	BaseThreadInitThunk [0x00007FFCF1FF4DE0+16]

	RtlUserThreadStart [0x00007FFCF3E1EC4B+43]

[ RUN      ] VerticalTabStripRootViewBrowserTest.DragOnCurrentTab

[11836:14924:0911/031921.800:WARNING:chrome_main_delegate.cc(609)] This is Chrome version 117.1.60.2 (not a warning)

[11836:14924:0911/031921.803:WARNING:policy_logger.cc(148)] :components\enterprise\browser\controller\chrome_browser_cloud_management_controller.cc(88) Could not create policy manager as CBCM is not enabled.

[11836:14924:0911/031921.832:ERROR:policy_logger.cc(154)] :components\enterprise\browser\controller\chrome_browser_cloud_management_controller.cc(163) Cloud management controller initialization aborted as CBCM is not enabled.

[11836:14924:0911/031921.854:WARNING:external_provider_impl.cc(513)] Malformed extension dictionary for extension: odbfpeeihdkbihmopkbjmoonfanlbfcl.  Key external_update_url has value "", which is not a valid URL.

..\..\brave\browser\ui\views\frame\vertical_tab_strip_root_view_browsertest.cc(142): error: Expected equality of these values:

  output_drag_op

    Which is: kNone

  ui::mojom::DragOperation::kCopy

    Which is: kCopy

Stack trace:

	VerticalTabStripRootViewBrowserTest_DragOnCurrentTab_Test::RunTestOnMainThread [0x00007FF70752CDF1+1553] (C:\ws\src\brave\browser\ui\views\frame\vertical_tab_strip_root_view_browsertest.cc:142)

	content::BrowserTestBase::ProxyRunTestOnMainThreadLoop [0x00007FF70CAB8E29+1001] (C:\ws\src\content\public\test\browser_test_base.cc:913)

	content::BrowserMainLoop::InterceptMainMessageLoopRun [0x00007FF709AF1BB2+112] (C:\ws\src\content\browser\browser_main_loop.cc:1042)

	content::BrowserMainLoop::RunMainMessageLoop [0x00007FF709AF1BFF+37] (C:\ws\src\content\browser\browser_main_loop.cc:1054)

	content::BrowserMainRunnerImpl::Run [0x00007FF709AF3B51+17] (C:\ws\src\content\browser\browser_main_runner_impl.cc:160)

	content::BrowserMain [0x00007FF709AEED0E+166] (C:\ws\src\content\browser\browser_main.cc:34)

	content::RunBrowserProcessMain [0x00007FF70AD8C05A+254] (C:\ws\src\content\app\content_main_runner_impl.cc:685)

	content::ContentMainRunnerImpl::RunBrowser [0x00007FF70AD8D1FC+408] (C:\ws\src\content\app\content_main_runner_impl.cc:1266)

	content::ContentMainRunnerImpl::Run [0x00007FF70AD8D02A+890] (C:\ws\src\content\app\content_main_runner_impl.cc:1120)

	content::RunContentProcess [0x00007FF70AD8B7A5+886] (C:\ws\src\content\app\content_main.cc:326)

	content::ContentMain [0x00007FF70AD8BA40+113] (C:\ws\src\content\app\content_main.cc:343)

	content::BrowserTestBase::SetUp [0x00007FF70CAB82F5+2437] (C:\ws\src\content\public\test\browser_test_base.cc:587)

	InProcessBrowserTest::SetUp [0x00007FF70B658E64+404] (C:\ws\src\chrome\test\base\in_process_browser_test.cc:503)



..\..\brave\browser\ui\views\frame\vertical_tab_strip_root_view_browsertest.cc(148): error: Value of: browser() ->tab_strip_model() ->GetWebContentsAt(0) ->GetURL() .EqualsIgnoringRef(url)

  Actual: false

Expected: true

Stack trace:

	VerticalTabStripRootViewBrowserTest_DragOnCurrentTab_Test::RunTestOnMainThread [0x00007FF70752CFFB+2075] (C:\ws\src\brave\browser\ui\views\frame\vertical_tab_strip_root_view_browsertest.cc:144)

	content::BrowserTestBase::ProxyRunTestOnMainThreadLoop [0x00007FF70CAB8E29+1001] (C:\ws\src\content\public\test\browser_test_base.cc:913)

	content::BrowserMainLoop::InterceptMainMessageLoopRun [0x00007FF709AF1BB2+112] (C:\ws\src\content\browser\browser_main_loop.cc:1042)

	content::BrowserMainLoop::RunMainMessageLoop [0x00007FF709AF1BFF+37] (C:\ws\src\content\browser\browser_main_loop.cc:1054)

	content::BrowserMainRunnerImpl::Run [0x00007FF709AF3B51+17] (C:\ws\src\content\browser\browser_main_runner_impl.cc:160)

	content::BrowserMain [0x00007FF709AEED0E+166] (C:\ws\src\content\browser\browser_main.cc:34)

	content::RunBrowserProcessMain [0x00007FF70AD8C05A+254] (C:\ws\src\content\app\content_main_runner_impl.cc:685)

	content::ContentMainRunnerImpl::RunBrowser [0x00007FF70AD8D1FC+408] (C:\ws\src\content\app\content_main_runner_impl.cc:1266)

	content::ContentMainRunnerImpl::Run [0x00007FF70AD8D02A+890] (C:\ws\src\content\app\content_main_runner_impl.cc:1120)

	content::RunContentProcess [0x00007FF70AD8B7A5+886] (C:\ws\src\content\app\content_main.cc:326)

	content::ContentMain [0x00007FF70AD8BA40+113] (C:\ws\src\content\app\content_main.cc:343)

	content::BrowserTestBase::SetUp [0x00007FF70CAB82F5+2437] (C:\ws\src\content\public\test\browser_test_base.cc:587)

	InProcessBrowserTest::SetUp [0x00007FF70B658E64+404] (C:\ws\src\chrome\test\base\in_process_browser_test.cc:503)



[11836:14924:0911/031924.595:WARNING:task_environment.cc(942)] ParallelExecutionFence is ineffective when ThreadPoolInstance is not managed by a TaskEnvironment.

Test fixtures should use a TaskEnvironment member or statically invoke TaskEnvironment::CreateThreadPool() + ThreadPoolInstance::Get()->StartWithDefaultParams() when the former is not possible.

[11836:14924:0911/031924.595:WARNING:task_environment.cc(942)] ParallelExecutionFence is ineffective when ThreadPoolInstance is not managed by a TaskEnvironment.

Test fixtures should use a TaskEnvironment member or statically invoke TaskEnvironment::CreateThreadPool() + ThreadPoolInstance::Get()->StartWithDefaultParams() when the former is not possible.

[11836:14924:0911/031924.596:WARNING:task_environment.cc(942)] ParallelExecutionFence is ineffective when ThreadPoolInstance is not managed by a TaskEnvironment.

Test fixtures should use a TaskEnvironment member or statically invoke TaskEnvironment::CreateThreadPool() + ThreadPoolInstance::Get()->StartWithDefaultParams() when the former is not possible.

[  FAILED  ] VerticalTabStripRootViewBrowserTest.DragOnCurrentTab, where TypeParam =  and GetParam() =  (2798 ms)

sangwoo108 · 2023-09-18T11:43:58Z

Sorry, didn't realize you pushed fix. Let me rebase and run CI again.

sangwoo108 · 2023-09-19T01:11:19Z

@thirumurugan-git , unfortunately, the fix didn't help us. I think we can dsiable the tests on Windows as I've seen that interactive tests for dnd on Windows flaky before. let me disable the tests and retry.

This reverts commit c5d348b.

sangwoo108 · 2023-09-19T02:15:15Z

@simonhong Could you take another look at this before merging this?

simonhong

++ 👍🏼

browser/ui/views/frame/vertical_tab_strip_root_view.h

browser/ui/views/tabs/brave_tab_container.cc

sangwoo108 · 2023-09-19T07:56:01Z

@thirumurugan-git I really appreciate your time and effort for this PR 👍

thirumurugan-git · 2023-09-19T14:40:20Z

@sangwoo108 I appreciate your help in diagnosing the issue and fixing it. Unfortunately, I was unable to resolve the issue myself as I do not have a Windows machine.

sangwoo108 reviewed Aug 21, 2023

View reviewed changes

sangwoo108 reviewed Aug 22, 2023

View reviewed changes

sangwoo108 requested a review from simonhong August 22, 2023 07:38

simonhong reviewed Aug 25, 2023

View reviewed changes

thirumurugan-git requested a review from a team as a code owner September 2, 2023 18:07

sangwoo108 reviewed Sep 5, 2023

View reviewed changes

ghost mentioned this pull request Sep 9, 2023

Vertical tab drag and drop support for links brave/brave-browser#32821

Closed

sangwoo108 approved these changes Sep 10, 2023

View reviewed changes

sangwoo108 mentioned this pull request Sep 10, 2023

Don't merge - this is a CI run for https://github.com/brave/brave-core/pull/19785 #20081

Merged

25 tasks

sangwoo108 force-pushed the vtab-drag-n-drop branch from 0e9fb35 to 27dc3ca Compare September 11, 2023 00:36

thirumurugan-git added 3 commits September 18, 2023 21:38

Vertical tab - drag&drop feature for link and text.

d6b624c

Vertical tab - drag & drop - Code refactoring.

56edfc4

Vertical tab - Drop arrow implementation.

312e419

thirumurugan-git and others added 6 commits September 18, 2023 21:39

Vertical tab - Cache drop indicator images.

67f2e73

Vertical tab - Code formatting

ae5f6cf

Address lint

519554d

Build fix trial

be19f08

Vertical tab - test drag and drop changes.

c5d348b

rebase

79cae2c

sangwoo108 force-pushed the vtab-drag-n-drop branch from dae75d2 to 79cae2c Compare September 18, 2023 13:07

sangwoo108 added 2 commits September 19, 2023 10:19

Revert "Vertical tab - test drag and drop changes."

94908f5

This reverts commit c5d348b.

Disable tests on Windows

5f4df19

sangwoo108 requested review from simonhong and removed request for a team September 19, 2023 02:15

simonhong approved these changes Sep 19, 2023

View reviewed changes

browser/ui/views/frame/vertical_tab_strip_root_view.h Outdated Show resolved Hide resolved

browser/ui/views/tabs/brave_tab_container.cc Outdated Show resolved Hide resolved

Address code review

07c7661

sangwoo108 enabled auto-merge September 19, 2023 06:51

sangwoo108 merged commit cc69cb1 into brave:master Sep 19, 2023
15 checks passed

ghost mentioned this pull request Sep 22, 2023

No New Tab when drag and drop elements like images into the vertical tab area brave/brave-browser#31016

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Vertical tab - drag&drop feature for link and text. #19785

Vertical tab - drag&drop feature for link and text. #19785

thirumurugan-git commented Aug 20, 2023 •

edited by sangwoo108

Loading

thirumurugan-git commented Aug 20, 2023

sangwoo108 left a comment

sangwoo108 commented Aug 21, 2023

thirumurugan-git commented Aug 22, 2023

sangwoo108 commented Aug 22, 2023

simonhong left a comment

thirumurugan-git commented Sep 2, 2023

sangwoo108 commented Sep 3, 2023

thirumurugan-git commented Sep 4, 2023

sangwoo108 commented Sep 4, 2023

thirumurugan-git commented Sep 4, 2023

sangwoo108 commented Sep 5, 2023

sangwoo108 commented Sep 5, 2023

sangwoo108 left a comment

thirumurugan-git commented Sep 10, 2023

sangwoo108 commented Sep 11, 2023

sangwoo108 commented Sep 11, 2023 •

edited

Loading

thirumurugan-git commented Sep 11, 2023

sangwoo108 commented Sep 11, 2023 •

edited

Loading

sangwoo108 commented Sep 18, 2023

sangwoo108 commented Sep 19, 2023

sangwoo108 commented Sep 19, 2023

simonhong left a comment

sangwoo108 commented Sep 19, 2023

thirumurugan-git commented Sep 19, 2023

Vertical tab - drag&drop feature for link and text. #19785

Vertical tab - drag&drop feature for link and text. #19785

Conversation

thirumurugan-git commented Aug 20, 2023 • edited by sangwoo108 Loading

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

thirumurugan-git commented Aug 20, 2023

sangwoo108 left a comment

Choose a reason for hiding this comment

sangwoo108 commented Aug 21, 2023

thirumurugan-git commented Aug 22, 2023

sangwoo108 commented Aug 22, 2023

simonhong left a comment

Choose a reason for hiding this comment

thirumurugan-git commented Sep 2, 2023

sangwoo108 commented Sep 3, 2023

thirumurugan-git commented Sep 4, 2023

sangwoo108 commented Sep 4, 2023

thirumurugan-git commented Sep 4, 2023

sangwoo108 commented Sep 5, 2023

sangwoo108 commented Sep 5, 2023

sangwoo108 left a comment

Choose a reason for hiding this comment

thirumurugan-git commented Sep 10, 2023

sangwoo108 commented Sep 11, 2023

sangwoo108 commented Sep 11, 2023 • edited Loading

thirumurugan-git commented Sep 11, 2023

sangwoo108 commented Sep 11, 2023 • edited Loading

sangwoo108 commented Sep 18, 2023

sangwoo108 commented Sep 19, 2023

sangwoo108 commented Sep 19, 2023

simonhong left a comment

Choose a reason for hiding this comment

sangwoo108 commented Sep 19, 2023

thirumurugan-git commented Sep 19, 2023

thirumurugan-git commented Aug 20, 2023 •

edited by sangwoo108

Loading

sangwoo108 commented Sep 11, 2023 •

edited

Loading

sangwoo108 commented Sep 11, 2023 •

edited

Loading