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

Split view MVP tasks #36986

Open
36 of 44 tasks
sangwoo108 opened this issue Mar 21, 2024 · 7 comments
Open
36 of 44 tasks

Split view MVP tasks #36986

sangwoo108 opened this issue Mar 21, 2024 · 7 comments
Assignees
Labels
OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. QA/No release-notes/exclude

Comments

@sangwoo108
Copy link

sangwoo108 commented Mar 21, 2024

https://www.figma.com/file/uiDbQ0ocpqiraWEeQVUyId/Tabs---Vertical-%2F-Split-%2F-Sidebar?type=design&node-id=35%3A6412&mode=dev

image

Tasks

  1. OS/Desktop QA/No priority/P3 release-notes/exclude
    sangwoo108
  2. OS/Desktop QA/No priority/P3 release-notes/exclude
    sangwoo108
  3. QA/No priority/P3 release-notes/exclude
    sangwoo108
  4. OS/Desktop QA/No priority/P3 release-notes/exclude split view
    sangwoo108
  5. OS/Desktop QA/No priority/P3 release-notes/exclude
    sangwoo108
  6. OS/Desktop QA/No priority/P3 release-notes/exclude split view
    sangwoo108
  7. OS/Desktop QA/No priority/P3 release-notes/exclude
    sangwoo108
  8. OS/Desktop QA/No priority/P3 release-notes/exclude
    sangwoo108
  9. OS/Desktop QA/No dev-concern release-notes/exclude
    sangwoo108
  10. OS/Desktop QA/No priority/P3 release-notes/exclude
    sangwoo108
  11. OS/Desktop QA/No priority/P3 release-notes/exclude split view
    sangwoo108
  12. OS/Desktop QA/No priority/P3 release-notes/exclude split view
    sangwoo108
  13. OS/Desktop QA/No priority/P3 release-notes/exclude split view
    sangwoo108
  14. OS/Desktop QA/No priority/P3 release-notes/exclude split view
    sangwoo108
  15. OS/Desktop QA/No priority/P3 release-notes/exclude split view
    sangwoo108
  16. OS/Desktop QA/No priority/P3 release-notes/exclude split view
    sangwoo108
  17. OS/Desktop QA/No priority/P3 release-notes/exclude split view
    sangwoo108
  18. OS/Desktop QA/No priority/P3 release-notes/exclude split view
    sangwoo108
  19. OS/Desktop QA/No priority/P3 release-notes/exclude split view
    sangwoo108
  20. OS/Desktop QA/No priority/P3 release-notes/exclude split view
    sangwoo108
  21. OS/Desktop QA/No priority/P3 release-notes/exclude split view
    sangwoo108
  22. OS/Desktop QA/No priority/P3 release-notes/exclude split view
    sangwoo108
  23. OS/Desktop QA Pass-Win64 QA/Yes release-notes/exclude split view
    sangwoo108
  24. OS/Desktop QA/No priority/P3 release-notes/exclude split view
    sangwoo108
  25. OS/Desktop QA/No priority/P3 release-notes/exclude split view
    sangwoo108
  26. OS/Desktop QA/No priority/P3 release-notes/exclude split view
    sangwoo108
  27. OS/Desktop QA/No priority/P3 release-notes/exclude split view
    sangwoo108
  28. OS/Desktop QA/No priority/P3 release-notes/exclude split view
    sangwoo108

TBD / Follow-ups

@bridiver
Copy link
Contributor

bridiver commented Apr 9, 2024

Similar to the follow up issue we should include more content here rather than just linking to Figma. This is currently completely opaque to anyone outside of Brave and I don't even know if everyone inside of Brave already has a Figma login or not.

@sangwoo108
Copy link
Author

I intentionally left figma link just in case. Asked @aguscruiz if it's okay to open our design public.

@aguscruiz
Copy link

Hi. This would probably be solved if we just add more screenshots to issues, right?

I try to add them sparingly because the problem with them is that they get outdated as soon as I make a tiny change. That's why I prefer figma links.

Regarding Figma access, everyone who has a @brave.com email has access to all of our Figma environment, so that shouldn't be a problem.

@bridiver
Copy link
Contributor

bridiver commented Apr 9, 2024

I try to add them sparingly because the problem with them is that they get outdated as soon as I make a tiny change. That's why I prefer figma links.

I think that actually makes the problem here worse because this PR represents a snapshot in time, but figma is changing. That makes it particularly difficult to come back to changes in the future to see what they were supposed to do. Remember that these issues attached to PRs are not just about what we need to do right now, they are a record of what we did and why for the future.

@sangwoo108 sangwoo108 changed the title Split view tasks Split view MVP tasks Apr 26, 2024
@TEMP-ad
Copy link

TEMP-ad commented May 30, 2024

Just throwing some general feedback of Split View progress.

  1. ${{\color{Orange}\huge{\text{ Add Menu button to split view separator 38584}}}}$: In Vertical tabs, when you enable Split view, and then you you expand or minimize it the VT, the button doesn't follow the split line, just resizing a little will fix it though.
    image

  2. ${{\color{green}\huge{\text{ Resized amount for split view tile should be kept per tile 38392}}}}$: This is still keeping the size of the split view per window, so for example, you enable Split view and change the size and then you split other tabs in the same window, it will show the same size, but not in a new window.
    I don't mind about the size being per window, because I think it would be good to have a have a 'reset' button to quickly reset everything to be 50/50 or have a way to specify it.

  3. ${{\color{green}\huge{\text{ Draw border for contents web views in split view mode 37804 }}}}$ this one looks nice, but doesn't let activate or move tabs by using the top of the tabs, like other normal tabs, so going from normal to split tabs can be weird.

  4. ${{\color{red}\huge{\text{ Add "Swap Split view position" 38656}}}}$: the context menu "Swap tab positions" can be used when you select a split tab and a non-split tab(s), which causes Browser to crash.

  5. ${{\color{BrickRed}\huge{\text{ alleviate flickering when switching tabs with split view 38691}}}}$: seems better but still happens sometimes, especially Vertical tabs, tested two twitch streams in a clean profile and it did a tiny flickering in VT but not in HT.
    I ran some tests, and the biggest issue is Custom Adblock rules too, for example, Outlook causes sometimes long black seconds while trying to redraw/layout the page in my main profile, and that's because I have a simple custom rule outlook.live.com###MainModule + div, without it, barely or no flickering but with it after my email is loaded, but with it, it can do even a 500ms long black screen. So there is something going on between Custom rules and the way it does it in split view.
    BTW, I modified the default list iodkpdagapdfkphljnddpjlldadblomo and added the same custom rule and the problem is not present BTW, so that's why I am 100% sure it is about the adblocking and what it does internally about the two engines it has and the merging and all that.

    PS: I added all my custom rules to the default list, and Twitch with all the hundred of rules I have made, doesn't have any super bad flickering anymore or the long black screen. The flicker is only visible a tiny fraction in Vertical Tabs but not Horizontal tabs. So I can double confirm my theory about custom cosmetic rules affecting the way this flickering affects split view tabs.
    So the problem is still present and almost unnoticeable in VT, but apparently (in my tests) I don't get it in HT anymore as much, sometimes but not as much as having a custom adblock rules that will cause long dark flickering.

1.68.61 Chromium: 126.0.6478.26 (Official Build) nightly (64-bit)
Windows 11 Version 23H2 (Build 22631.3672)

@sangwoo108
Copy link
Author

Hi @TEMP-ad , Thanks for reporting.

As for the #3 one, I guess you mean the background around the tabs when they're in a split view, not the border around the web views. Let me check if we can do something.

And #5 would be tricky to tell what's going on. As far as I can tell, the long loading black screen with custom rule could be due to renderer or GPU part. If that's the case, it'd be hard to fix it perfectly.

@TEMP-ad
Copy link

TEMP-ad commented Jun 1, 2024

tabn@sangwoo108

  1. yeah, since you added the background around the split views, it is not possible to click and go to the tabs or move the split view, it will move the window, so it is inconsistent with how the other tabs work.

    Recording.2024-05-31.221443.mp4
  2. Also, seems like the background color is the same as the titlebar in normal mode in Horizontal tab (you can see it in the video I posted) in private windows it looks better, but it is only in VT where the background is noticeable and looks great. That's also a tiny little issue.

    image
    image

    vs

    image
    image

  3. I also got a tiny little issue that I forgot to mention, it has to do with Cycle through the most recently used tabs with Ctrl-Tab; it doesn't work properly with Split view tabs, especially across different split view tabs.

    Recording.2024-05-31.190734.mp4
    • It's hard to tell what the video is about, but first I click on tab 2nd and then 3rd tab (so different split views) and then you see it goes to the 4th tab, now I did the opposite from 3 to 2, and it goes to the 1st tab.

    • Then it's hard to tell what I do, because you can't see when I press ctrl+tab, but it does what it is suppose to do but I have to press ctrl+tab twice, to go tom 4th tab to 2nd tab, then I have to press twice ctrl+tab for it to go back to the 4th tab.

    • And same from 1st to 3rd tab. and then trying to go back to the 1st tab.

    • When it comes to split view tabs vs nonsplit view tabs,

      • And clicking on a non split view tab, 5th tab, then I click on the 4th tab, and it will require require also two ctrl+tab to do the job.

      • And then the last part of the video is me showing how I click on the 5th tab (non split view) and then 3rd tab, but then it jumps to the 4th tab before going to the 5th tab which is the 'recent one' so basically if you go from one non split tab and click on the left tab of a split view it will go through both tabs when ctrl+tab, but if you click on the left tab of the split it will require two ctrl+tab to properly cycle with the recent tab.

It is a tiny little miss behavior but forgot to report it, I am sure it will make more sense when you enable it (in brave://settings/braveContent)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. QA/No release-notes/exclude
Projects
None yet
Development

No branches or pull requests

4 participants