Skip to content

Conversation

rrahir
Copy link
Collaborator

@rrahir rrahir commented Sep 23, 2025

How to reproduce:

  • Open the ribbon menu
  • select "insert"
  • scroll the menu completely
  • select "Functions"

The scroll is persistent from the previous menu and we arrive in the Functions menu already scrolled, issue that is highlighted by the fact that scrollbars are hidden by defaut on mobile browsers.

This revision resets the scroll when ;oving from one menu section to the other.

[FIX] RibbonMenu: hint the scrollable menu

Currently, it is not quite clear that the ribbon menu can be scrolled as
the scrollbars are not displayed by default on a mobile browser (or on
some desktop browsers like firefox).

This revision adds some style that will hint the possibility to scroll
either to the bottom or top of the menu depending on the current
scrolling value.

Task: 5106796

Description:

description of this task, what is implemented and why it is implemented that way.

Task: 5106796

review checklist

  • feature is organized in plugin, or UI components
  • support of duplicate sheet (deep copy)
  • in model/core: ranges are Range object, and can be adapted (adaptRanges)
  • in model/UI: ranges are strings (to show the user)
  • undo-able commands (uses this.history.update)
  • multiuser-able commands (has inverse commands and transformations where needed)
  • new/updated/removed commands are documented
  • exportable in excel
  • translations (_t("qmsdf %s", abc))
  • unit tested
  • clean commented code
  • track breaking changes
  • doc is rebuild (npm run doc)
  • status is correct in Odoo

@robodoo
Copy link
Collaborator

robodoo commented Sep 23, 2025

Pull request status dashboard

How to reproduce:

- Open the ribbon menu
- select "insert"
- scroll the menu completely
- select "Functions"

The scroll is persistent from the previous menu and we arrive in the
`Functions` menu already scrolled, issue that is highlighted by the fact
that scrollbars are hidden by defaut on mobile browsers.

This revision resets the scroll when ;oving from one menu section to the
other.

Task: 5106796
@rrahir rrahir force-pushed the saas-18.4-fix-ribbon-menu-scroll-rar branch from 8548645 to 468e539 Compare September 23, 2025 13:50
Copy link
Contributor

@hokolomopo hokolomopo left a comment

Choose a reason for hiding this comment

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

👋

test("Spam click on the arrow button scrolls a lot", async () => {
let scrollTo = 0;
//@ts-ignore - scrollTo is not defined in JSDOM
sheetListEl.scrollTo = (arg: ScrollToOptions) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is scrollTo in JSDOM now ? 😮

Edit: NEVERMIND you added it in the jest.setup. For a second, I was hoping JSDOM was getting good 😢

.o-ribbon-menu-wrapper {
max-height: 100%;

&.scroll-top::before {
Copy link
Contributor

Choose a reason for hiding this comment

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

css seems a bit broken, the left/right side of the menu also have a shadow for me ...

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

apparently this behaves dfferently on miobile devices (perharps per browser/os ) i will change the strategy

Currently, it is not quite clear that the ribbon menu can be scrolled as
the scrollbars are not displayed by default on a mobile browser (or on
some desktop browsers like firefox).

This revision adds some style that will hint the possibility to scroll
either to the bottom or top of the menu depending on the current
scrolling value.

Task: 5106796
@rrahir rrahir force-pushed the saas-18.4-fix-ribbon-menu-scroll-rar branch from 468e539 to 9165a5a Compare September 29, 2025 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants