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 "$tab.activate.tab" called after renavigate wrongly #3160

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

mvorisek
Copy link
Contributor

fix #3156

Currently, the only "fetch" is invoked after https://github.com/fomantic/Fomantic-UI/blob/13b76ee714/src/definitions/modules/tab.js#L367 thus the "navigation" should be always active when the tab "is beiing activated and loading".

@mvorisek
Copy link
Contributor Author

The impl. is working.

I still have one question - https://github.com/fomantic/Fomantic-UI/blob/13b76ee714/src/definitions/modules/tab.js#L482 - what is activeTabPath variable for?

It seems it is set only within the actual tab, not set of tabs, and in my testing, it was always set to the fetched tab even if the tab was changed until loaded, ie. the condition was always true.

I am not sure what the variable is for, IMO, it is useless and wrong, as other code is checking if the navigation/tab is active always using DOM - https://github.com/fomantic/Fomantic-UI/blob/13b76ee714/src/definitions/modules/tab.js#L550-L554.

@mvorisek mvorisek changed the title Fix "$tab.activate.tab" called after renavigate Fix "$tab.activate.tab" called after renavigate wrongly Jan 31, 2025
@mvorisek
Copy link
Contributor Author

mvorisek commented Feb 5, 2025

Hi @lubber-de, might I have your review?

@lubber-de
Copy link
Member

Hi @lubber-de, might I have your review?

Quickly looking at the logic, the activeTabPath should be set to the latest clicked tab, so whenever it changes in between the ajax request, tabPath should not be equal to activeTabPath anymore, thus isn't active, so the additional check should not be needed.
However, given your examples, it doesn't seem to work as expected as especially the wrong tab gets updated.

Yes, also the later check for the active class seems not quite right as you pointed out.

I have to debug all of this using some simplfied testcase myself. I suspect something else / more we need to fix.

@lubber-de lubber-de added the state/awaiting-investigation Anything which needs more investigation label Feb 5, 2025
@mvorisek
Copy link
Contributor Author

I am marking this PR as draft to prevent unintentional merge.

@mvorisek mvorisek marked this pull request as draft February 15, 2025 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state/awaiting-investigation Anything which needs more investigation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Tab] Async context must not replace different tab
2 participants