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

fix(core/menu-about): unwanted behavior on fast tab change #933

Merged
merged 8 commits into from
Feb 8, 2024

Conversation

matthiashader
Copy link
Collaborator

@matthiashader matthiashader commented Nov 22, 2023

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build (yarn build) was run locally and any changes were pushed
  • Unit tests (yarn test) were run locally and passed
  • Visual Regression Tests (yarn visual-regression) were run locally and passed
  • Linting (npm lint) was run locally and passed

Pull request type

Please check the type of change your PR introduces:

  • Bug fix
  • Feature
  • Refactoring (no functional changes, no API changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

GitHub Issue Number: #879

What is the new behavior?

Removed the timeout and implemented a single source of truth for menu-settings and menu-about.

Does this introduce a breaking change?

  • Yes
  • No

Testing

Manual testing.

Other information

@danielleroux danielleroux added this to the 2.1.0 milestone Dec 21, 2023
Copy link
Collaborator

@danielleroux danielleroux left a comment

Choose a reason for hiding this comment

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

PR fixes only menu-about but menu-settings contains a similar implementation. Was this also checked?

//EDIT Can you add more information to the PR description

@matthiashader
Copy link
Collaborator Author

PR fixes only menu-about but menu-settings contains a similar implementation. Was this also checked?

//EDIT Can you add more information to the PR description

I tried to reproduce it, but could not reproduce the error in the menu-settings:
async delay(ms) {
return new Promise((resolve) => setTimeout(resolve, ms));
}

async fastChange() {
const menuSettings = document.querySelector('ix-menu-settings');
const menuSettingsShadowRoot = menuSettings.shadowRoot;
const menuSettingsItems =
menuSettingsShadowRoot.querySelectorAll('ix-tab-item');

for (let i = 0; i < 100; i++) {
  for (const elements of Array.from(menuSettingsItems)) {
    await elements.click();
    await this.delay(10);
  }
  console.log(i);
}

}

render() {
return (






Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed
diam nonumy eirmod tempor invidunt ut labore et dolore magna
aliquyam erat, sed diam voluptua. At vero eos et accusam et
justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea
takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum
dolor sit amet, consetetur sadipscing elitr, sed diam nonumy
eirmod tempor invidunt ut labore et dolore magna aliquyam erat,
sed diam voluptua. At vero eos et accusam et justo duo dolores
et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus
est Lorem ipsum dolor sit amet.


Stet clita kasd gubergren, no sea takimata sanctus est Lorem
ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur
sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut
labore et dolore magna aliquyam erat, sed diam voluptua. At vero
eos et accusam et justo duo dolores et ea rebum. Stet clita kasd
gubergren, no sea takimata sanctus est Lorem ipsum dolor sit
amet. Duis autem vel eum iriure dolor in hendrerit in vulputate
velit esse molestie consequat, vel illum dolore eu feugiat nulla
facilisis at vero eros et accumsan et iusto odio dignissim qui
blandit praesent luptatum zzril delenit augue duis dolore te
feugait nulla facilisi. Lorem ipsum dolor sit amet,




<ix-button
onClick={() => {
this.fastChange();
}}
>
PUSH ME

@jul-lam jul-lam self-assigned this Jan 16, 2024
@danielleroux danielleroux added the pull request affects patch version The pull request affects only patch version label Jan 25, 2024
@nuke-ellington nuke-ellington merged commit 1558e92 into main Feb 8, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pull request affects patch version The pull request affects only patch version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug in ix-menu-about when rapidly changing tabs with a lot of data
4 participants