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

Notebookbar: main-nav: Fix last child being glued to the edge #10896

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pedropintosilva
Copy link
Contributor

Before this commit the .main-nav's width had bigger width than the
webview due to borders not being factored in. This was particular
visible for integrators that have the close button toggled off and
that have no "EnableShare" where the last sidebar icon was "glued" the
right edge of the web view.

Cases where this is possible to test:

  • Using the make run Edit mode urls in Tabbed view
  • Places where Collabora Online is integrated with close and
    share buttons. Example: Moddle

Signed-off-by: Pedro Pinto Silva [email protected]
Change-Id: Ie189ed780080cda69d3d41e425a047c45d0e69e9

Copy link
Contributor

@Darshan-upadhyay1110 Darshan-upadhyay1110 left a comment

Choose a reason for hiding this comment

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

Tested, Working good. Thanks for the fix!

Darshan-upadhyay1110

This comment was marked as duplicate.

@Darshan-upadhyay1110 Darshan-upadhyay1110 enabled auto-merge (rebase) January 13, 2025 14:08
@pedropintosilva
Copy link
Contributor Author

these tests seem to fail with or without this PR

@pedropintosilva pedropintosilva force-pushed the private/pedro/fix-main-nav-width branch from f12c2a4 to 498bcba Compare January 14, 2025 08:39
@pedropintosilva
Copy link
Contributor Author

I have rebased and it passes the tests locally so, force pushed.

@pedropintosilva
Copy link
Contributor Author

make -C cypress_test run-desktop spec=writer/navigator_spec.js

fails for me locally, it seems there was a change that got in before my PR and that is somehow breaking the Visual Regression Plugin

Base image Actual Image
base-image actual-image
Peek.2025-01-14.15-42.mp4

@pedropintosilva
Copy link
Contributor Author

Also this branch is missing the multi-use cypress fix: 188ca7a

@Darshan-upadhyay1110
Copy link
Contributor

@pedropintosilva need to rebase this PR. I have merged the Navigator PR: #10936

@pedropintosilva pedropintosilva force-pushed the private/pedro/fix-main-nav-width branch from 09d0885 to 59527c7 Compare January 15, 2025 11:43
@pedropintosilva
Copy link
Contributor Author

now the test that sometimes fails is make -C cypress_test run-desktop spec=writer/scrolling_spec.js but then again it also sometimes fails when I checkout master 🤷‍♂️

@pedropintosilva
Copy link
Contributor Author

Ok so, I was able to reproduce this cypress fail manually with (private/pedro/fix-main-nav-width) or without (master) this commit by:

  1. running online and open cypress_test/data/desktop/writer/scrolling.odt document with 1000 x 600 window dimensions

  2. Click the status bar's right scroll button/indicator till see the zoom controls

  3. Change to 40%

  4. Press ctrl + home

  5. Press Pagedown

    • The StatePageNumber status says Page 1 of 4 which is incorrect it should be Page 2 (Cypress is correct there is a problem in the behaviour): when pressing pagedown the text cursor should be moved to the next page, that's what I expect and it's what happens in other office suites image
  6. Press Pagedown

  7. The StatePageNumber status says Page 2 of 4 which is incorrect it should be Page 3 (Cypress is correct there is a problem in the behaviour): when pressing pagedown the text cursor should be moved to the next page, that's what I expect and it's what happens in other office suites

    • image
cy:command ✘ assert expected **Page 3 of 4** to be one of **[ Page 2 of 4 ]**
      cy:command   cGet	#zoom-dropdown
      cy:command   contains	.ui-combobox-entry, 40
      cy:command   click	{force: true}
          cy:log   >> shouldHaveZoomLevel - start
      cy:command   cGet	#toolbar-down #zoom .unolabel
      cy:command   assert	expected **<label.ui-content.unolabel>** to have text **'40'**
          cy:log   << shouldHaveZoomLevel - end
          cy:log   << selectZoomLevel - end
          cy:log   >> typeIntoDocument - start
      cy:command   cGet	div.clipboard
      cy:command   type	{ctrl}{home}, {force: true}
        cons:log   ==== debug.html receiveMessage: {"MessageId":"FollowUser_Changed","SendTime":1736970524795,"Values":{"FollowedViewId":4,"IsFollowUser":true,"IsFollowEditor":false}}
        cons:log   ==== debug.html receiveMessage: {"MessageId":"FollowUser_Changed","SendTime":1736970524806,"Values":{"FollowedViewId":4,"IsFollowUser":true,"IsFollowEditor":false}}
        cons:log   ==== debug.html receiveMessage: {"MessageId":"FollowUser_Changed","SendTime":1736970524807,"Values":{"FollowedViewId":4,"IsFollowUser":true,"IsFollowEditor":false}}
        cons:log   ==== debug.html receiveMessage: {"MessageId":"FollowUser_Changed","SendTime":1736970524818,"Values":{"FollowedViewId":4,"IsFollowUser":true,"IsFollowEditor":false}}
          cy:log   << typeIntoDocument - end
      cy:command   cGet	#StatePageNumber
      cy:command   invoke	.text()
      cy:command   assert	expected **Page 1 of 4** to be one of **[ Page 1 of 4 ]**
          cy:log   >> pressKey - start
          cy:log   >> typeIntoDocument - start
      cy:command   cGet	div.clipboard
      cy:command   type	{pagedown}, {force: true}
        cons:log   ==== debug.html receiveMessage: {"MessageId":"FollowUser_Changed","SendTime":1736970524878,"Values":{"FollowedViewId":4,"IsFollowUser":true,"IsFollowEditor":false}}
        cons:log   ==== debug.html receiveMessage: {"MessageId":"FollowUser_Changed","SendTime":1736970524879,"Values":{"FollowedViewId":4,"IsFollowUser":true,"IsFollowEditor":false}}
          cy:log   << typeIntoDocument - end
          cy:log   >> typeIntoDocument - start
      cy:command   cGet	div.clipboard
      cy:command   type	{pagedown}, {force: true}
        cons:log   ==== debug.html receiveMessage: {"MessageId":"FollowUser_Changed","SendTime":1736970524946,"Values":{"FollowedViewId":4,"IsFollowUser":true,"IsFollowEditor":false}}
        cons:log   ==== debug.html receiveMessage: {"MessageId":"FollowUser_Changed","SendTime":1736970524947,"Values":{"FollowedViewId":4,"IsFollowUser":true,"IsFollowEditor":false}}
          cy:log   << typeIntoDocument - end
          cy:log   << pressKey - end
      cy:command   cGet	#StatePageNumber
      cy:command   invoke	.text()
      cy:command   assert	expected **Page 3 of 4** to be one of **[ Page 2 of 4 ]**
                    Actual: 	"Page 3 of 4"
                    Expected: 	["Page 2 of 4"]

Before this commit the .main-nav's width had bigger width than the
webview due to borders not being factored in. This was particular
visible for integrators that have the close button toggled off and
that have no "EnableShare" where the last sidebar icon was "glued" the
right edge of the web view.

Cases where this is possible to test:
- Using the make run Edit mode urls in Tabbed view
- Places where Collabora Online is integrated with close and
share buttons. Example: Moddle

Signed-off-by: Pedro Pinto Silva <[email protected]>
Change-Id: Ie189ed780080cda69d3d41e425a047c45d0e69e9
@pedropintosilva pedropintosilva force-pushed the private/pedro/fix-main-nav-width branch from 59527c7 to a4863e3 Compare January 16, 2025 16:10
@eszkadev eszkadev added the draft label Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: To Test
Development

Successfully merging this pull request may close these issues.

3 participants