Skip to content

Conversation

@hokolomopo
Copy link
Contributor

Description

Since the topbar refactor, a new render is triggered at every single click event, no matter where we click. This breaks every input with t-att-value, their content is reset at each click.

Task: 4737827

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 Apr 18, 2025

Pull request status dashboard

Comment on lines +160 to +163
jest.useFakeTimers();
model = new Model(data, { external: { geoJsonService: mockGeoJsonService } });
jest.advanceTimersByTime(DEBOUNCE_TIME + 10); // wait for the debounce of session.move
jest.useRealTimers();
Copy link
Contributor Author

@hokolomopo hokolomopo Apr 18, 2025

Choose a reason for hiding this comment

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

I don't understand how this is not more of a problem. Inside every single test there is parasitic renders happening in the middle of the test because of debounced session.move. Idk how we don't have more indeterministic tests ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually scratch that, this is a problem. I'm 80% sure that's why the composer component tests keep randomly failing on my machine ....

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't recall if we found a idea to solve this? I only recall the ideas:

  • skipping the first debounce
  • having an overall mock of the session in the tests but it lead to the import nightmare...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in another PR :) (In master, because it needed some tweaks in the file structure)

#6162

@hokolomopo hokolomopo force-pushed the master-onclick-parasitic-render-adrm branch from 5f8092d to c5ffee5 Compare April 18, 2025 13:23
@hokolomopo hokolomopo changed the base branch from master to saas-18.3 April 28, 2025 11:46
Copy link
Collaborator

@rrahir rrahir left a comment

Choose a reason for hiding this comment

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

🚚 50/50 (I hope) on the comments relevance

Comment on lines +160 to +163
jest.useFakeTimers();
model = new Model(data, { external: { geoJsonService: mockGeoJsonService } });
jest.advanceTimersByTime(DEBOUNCE_TIME + 10); // wait for the debounce of session.move
jest.useRealTimers();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't recall if we found a idea to solve this? I only recall the ideas:

  • skipping the first debounce
  • having an overall mock of the session in the tests but it lead to the import nightmare...

Since the topbar refactor, a new render is triggered at every single
click event, no matter where we click. This breaks every input with
`t-att-value`, their content is reset at each click.

Task: 4737827
@hokolomopo hokolomopo force-pushed the master-onclick-parasitic-render-adrm branch from c5ffee5 to cbf59c1 Compare April 30, 2025 08:42
@rrahir
Copy link
Collaborator

rrahir commented Apr 30, 2025

@robodoo r+

robodoo pushed a commit that referenced this pull request Apr 30, 2025
Since the topbar refactor, a new render is triggered at every single
click event, no matter where we click. This breaks every input with
`t-att-value`, their content is reset at each click.

closes #6145

Task: 4737827
Signed-off-by: Rémi Rahir (rar) <[email protected]>
@robodoo robodoo closed this Apr 30, 2025
@fw-bot fw-bot deleted the master-onclick-parasitic-render-adrm branch May 7, 2025 09:00
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