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

[Tab] Make auto activation of tabs optional #1488

Merged

Conversation

lubber-de
Copy link
Member

@lubber-de lubber-de commented May 29, 2020

Description

The implemented auto activation of tabs when no active tab is found #977 #1025 lead into some situations where a manual set of the active tab does not work anymore and always the last tab was selected.
This especially happens when tabs are initialized separately. Such situations need the old until FUI 2.7.7 which did not check for existing active tabs at all

This PR adds a new option autoTabActivation (default true to avoid being a breaking change within the 2.8.x branch)
When set to false the behavior is the same as until 2.7.7
When set to a string and no active tab exists, it will use that string as the path name for the tab to be activated.

Testcase

The third tab is supposed to be active

Broken

The last initiated tab is selected
https://jsfiddle.net/z632cfq5

Fixed

The previous selected tab stays selected
https://jsfiddle.net/z632cfq5/1/

Closes

#1255
#1188
atk4/ui#1044

@lubber-de lubber-de added type/bug Any issue which is a bug or PR which fixes a bug type/feat Any feature requests or improvements lang/javascript Anything involving JavaScript state/awaiting-reviews Pull requests which are waiting for reviews labels May 29, 2020
@lubber-de lubber-de added this to the 2.8.x milestone May 29, 2020
@lubber-de lubber-de added the state/awaiting-docs Pull requests which need doc changes/additions label May 29, 2020
exoego
exoego previously approved these changes May 29, 2020
Copy link
Contributor

@exoego exoego left a comment

Choose a reason for hiding this comment

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

LGTM

@lubber-de
Copy link
Member Author

Please review again, i enhanced the usage of the new parameter, so you could also provide a string to switch to a defined tab
https://jsfiddle.net/ntz2eyf5/

@lubber-de lubber-de requested a review from exoego May 29, 2020 22:15
@lubber-de lubber-de added state/has-docs A issue/PR which requires documentation changes and has the corresponding PR open in the docs repo and removed state/awaiting-docs Pull requests which need doc changes/additions labels May 29, 2020
Copy link
Contributor

@exoego exoego left a comment

Choose a reason for hiding this comment

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

Is it intended to pass autoTabActivations even when null or false?

@lubber-de
Copy link
Member Author

Yes it's in intented to skip the check if it's null/false.
That was the main reason for this PR to make it possible to skip the check at all to gain the old behavior as of FUI 2.7.7.
See the linked issues why that it necessary in some situations

Copy link
Contributor

@exoego exoego left a comment

Choose a reason for hiding this comment

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

LGTM

@lubber-de lubber-de merged commit 9f24338 into fomantic:develop May 31, 2020
@lubber-de lubber-de deleted the fix/1255/optionToSkipActiveTabCheck branch May 31, 2020 23:49
@lubber-de lubber-de removed this from the 2.8.x milestone May 31, 2020
@lubber-de lubber-de added this to the 2.8.5 milestone May 31, 2020
@lubber-de lubber-de removed the state/awaiting-reviews Pull requests which are waiting for reviews label Jun 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang/javascript Anything involving JavaScript state/has-docs A issue/PR which requires documentation changes and has the corresponding PR open in the docs repo type/bug Any issue which is a bug or PR which fixes a bug type/feat Any feature requests or improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants