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 #3499 #4405

Merged
merged 3 commits into from
Apr 14, 2024
Merged

Fix #3499 #4405

merged 3 commits into from
Apr 14, 2024

Conversation

MarcoAllenspach
Copy link
Contributor

fixed #3499

image

Remove sentence but not hint

frontend/src/locales/de.json Show resolved Hide resolved
@manuelmeister manuelmeister added the deploy! Creates a feature branch deployment for this PR label Jan 9, 2024
@manuelmeister manuelmeister self-requested a review January 9, 2024 15:28
Copy link

github-actions bot commented Jan 9, 2024

Feature branch deployment currently inactive.

If the PR is still open, you can add the deploy! label to this PR to trigger a feature branch deployment.

Copy link
Member

@manuelmeister manuelmeister left a comment

Choose a reason for hiding this comment

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

I think it would be better to remove the tooltip and just use the hint instead if readonlyRole is true, else we omit the hint.

@MarcoAllenspach
Copy link
Contributor Author

@manuelmeister
Like this?
image

I don't think this is better because the user can press the button and don't know why it doesn't works if he don't read carefully...
Or not?

@MarcoAllenspach
Copy link
Contributor Author

@manuelmeister see my last comment

@manuelmeister
Copy link
Member

@MarcoAllenspach
I recommend removing the tooltip around the readonlyRole select and instead readd the hint to the select. I think it is still good to remove the hint on the non-readonlyRole select, as it is only relevant if you edit the last manager.
This approach provides a clearer user experience, as it directly explains within the textfield why it is not editable, rather than relying on a tooltip.

Regarding the tooltip on the button, I suggest keeping it. Apologies for any confusion caused by my previous response.

@BacLuc BacLuc removed the deploy! Creates a feature branch deployment for this PR label Feb 17, 2024
@BacLuc
Copy link
Contributor

BacLuc commented Apr 14, 2024

If @MarcoAllenspach or someone else wants to continue, they are free to reopen it.
The proposal is also referenced in the issue.

@BacLuc BacLuc closed this Apr 14, 2024
@manuelmeister manuelmeister reopened this Apr 14, 2024
@manuelmeister manuelmeister added this pull request to the merge queue Apr 14, 2024
Merged via the queue into ecamp:devel with commit 2aaedfa Apr 14, 2024
32 checks passed
This was referenced Apr 28, 2024
@MarcoAllenspach
Copy link
Contributor Author

@manuelmeister
Sorry was really busy the last time
I recommend removing the tooltip around the readonlyRole select and instead readd the hint to the select. I think it is still good to remove the hint on the non-readonlyRole select, as it is only relevant if you edit the last manager.
This approach provides a clearer user experience, as it directly explains within the textfield why it is not editable, rather than relying on a tooltip.

So its like in my picture on the comment from January 9te?

Or do i understand it wrong?

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.

Team: Remove role min required admins in readonly & creation mode
3 participants