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 context manager allowing to switch to 'No Task' #451

Merged
merged 2 commits into from
Sep 19, 2019

Conversation

BigRoy
Copy link
Collaborator

@BigRoy BigRoy commented Sep 18, 2019

Issue

It's possible to have the Context Manager switch the AVALON_TASK to "No Task" whereas it should disallow that entry to be active task.

Reported here

What's changed?

Previously the selected task was retrieved from the view as the view.currentIndex() however that also allows non-selectable items to be active and returned. Now this is updated to solely using the actually selected rows from the view to ensure the Task is selected and not just the current.

This way you cannot switch to the Task, because the "No Task" item is unselectable in the model.

@BigRoy BigRoy requested a review from davidlatwe September 18, 2019 15:14
@BigRoy
Copy link
Collaborator Author

BigRoy commented Sep 18, 2019

This could also be adapted to disabling the "set context" button altogether when there's no selected asset or task. Will adapt this PR.

@BigRoy
Copy link
Collaborator Author

BigRoy commented Sep 18, 2019

Added the enable/disabled state toggle for the Apply button with 2620c29

Example:

contextmanager_enable_disable_apply_btn

Copy link
Collaborator

@davidlatwe davidlatwe left a comment

Choose a reason for hiding this comment

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

Awesome :D

@mottosso
Copy link
Contributor

Great work. Nice and concise. Merge when happy. 👍

@davidlatwe davidlatwe merged commit fbd00ad into getavalon:master Sep 19, 2019
@BigRoy BigRoy deleted the fix_switch_to_no_task branch January 25, 2021 14:52
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