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

[24.0] do not show copy button when editing #17895

Merged
merged 2 commits into from
Apr 4, 2024

Conversation

martenson
Copy link
Member

@martenson martenson commented Apr 3, 2024

backport plus an addition to #17888

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@martenson martenson added this to the 24.0 milestone Apr 3, 2024
@@ -84,6 +84,7 @@ function onCopyOut() {
</BButton>

<BButton
v-if="!editing"
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it'd be better to just disable the button ?

Copy link
Member Author

@martenson martenson Apr 3, 2024

Choose a reason for hiding this comment

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

I generally think it better to hide things that are not relevant to the current view. And when editing slug copying of the full url is not relevant.

Copy link
Contributor

Choose a reason for hiding this comment

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

In this case, there is not too much of a difference, but there are clear pros and cons for both options so we shouldn't always prioritize one or the other, but rather choose on a case-by-case basis.
Here is an article without the aim to bikeshed too much 😜 https://uxpsychology.substack.com/p/hidden-vs-disabled-states

Copy link
Member Author

Choose a reason for hiding this comment

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

I see both sides and my preference was in the commit. However since there are two votes for the other option I added cd7de55

Copy link
Contributor

@davelopez davelopez left a comment

Choose a reason for hiding this comment

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

Thank you! I prefer this option because the UI is not very cluttered here and there is a bit less "movement" on the screen when interacting 👍

@martenson martenson merged commit 1cd9752 into galaxyproject:release_24.0 Apr 4, 2024
28 checks passed
@martenson martenson deleted the backp branch April 4, 2024 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants