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

Add option to disable onClose/onClick functionality #872

Merged
merged 4 commits into from
Jul 31, 2024

Conversation

thecalcc
Copy link
Contributor

SDBELGA-847

@tomaskikutis
Copy link
Member

What's the purpose of the PR? What use-case are you handling?

@thecalcc
Copy link
Contributor Author

thecalcc commented Jul 22, 2024

What's the purpose of the PR? What use-case are you handling?

Handling the case when a widget is pinned. If it is, we should disable specific onClick handlers. In this case the handles responsible for closing are the ones I've modified in this PR. Besides that the style related changes are because once pinned, when you hover on the sidebar, no close button should be shown.

@tomaskikutis
Copy link
Member

onClose is already optional. If you don't pass it or pass undefined, the close button will not be rendered. Same with hovering - if it's not rendered it won't appear in hover state. Handle this on client-core side. Is there anything else that can't be handled on client core and needs changes here?

@thecalcc
Copy link
Contributor Author

thecalcc commented Jul 24, 2024

onClose is already optional. If you don't pass it or pass undefined, the close button will not be rendered. Same with hovering - if it's not rendered it won't appear in hover state. Handle this on client-core side. Is there anything else that can't be handled on client core and needs changes here?

Removing a button if it's not needed in not the way to go about it I think. We need it to be present in the UI & make it disabled because of visual feedback the users need. Removing it from the UI completely would confuse the customer a lot. Besides this, for the second point - onHover is there and it'll stay, it's controlled through activeTab, I have to make stuff up to make it work, needs to be handled in UIF to be proper.

@tomaskikutis
Copy link
Member

Removing it from the UI completely would confuse the customer a lot

How about not disabling closing and if user has it pinned, but still wants to close it - it would unpin and close as expected?

@thecalcc thecalcc merged commit 3a3b533 into superdesk:develop Jul 31, 2024
5 checks passed
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.

2 participants