Add yank button to the web UI #2623
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @sgrif (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
thiagoarrais
force-pushed
the
yank
branch
2 times, most recently
from
9178683 to
2663aa5
Compare
thiagoarrais
changed the title
|
Tagging @carols10cents (original issue author), @locks (original issue assignee) and @sgrif (pull request assignee) for review. Folks, the crates.io codebase is very nice to work with. Special thanks for the folks that wrote the contributing guide. Just let me know if you need anything else. |
|
Hey @thiagoarrais thanks for the PR! Several volunteers on the team are on vacation right now, so it may take a while to get this merged, but I've tried this out locally and have a few initial suggestions. On the page showing all versions of a crate, we can probably get rid of the green arrow. This is a small click target (right next to the new buttons) and links to the same page as the version text on the left.
On the crate pages, I'm not sure about having this shown for each version:
I think instead of showing the button here, we could show this at the top of the version-specific pages (so |
Since it no longer appears in the screen-real-estate-impaired latest versions short list
thiagoarrais
changed the title
|
Thanks for the review, @jtgeibel! Regarding the idea of moving the button to the page header. This is what I came up with so far:
I really like this idea. I've chose to place the yank button next to the version number. I think this makes it clearer that this is an action specific to that version, as opposed to an overall crate action (like the follow button). But I haven't got a ton of design skills and the button now seems a little out of place. Any ideas for making this better? By the way, I've also removed the green arrow from the owner view. |
|
Here are some options for the positioning of the yank button. This PR uses the first option for the time being, but it still needs some work. I'm also open to other options, of course. What do you folks think? Option A: Small button next to the version number
This is the current approach written into this PR. I think it makes clear(er) that the action refers to the specific version. We may assume that crate authors are already used to yanking versions (and not full-blown crates), but I think being more specific here avoids some confusion. That said, this needs some layouting work. I can't pinpoint exactly why, but the button as is looks a little out of place to me. Option B: Large button next to the follow button
This one avoids the positioning problem, but it may be confusing for crate authors. It may look like the yank action refers to the crate itself, not the specific version. This is aggravated by the presence of the follow button, that does trigger an action referring to the crate, not the version. Which brings us to... Option C: Large button replacing the follow button
This is a variation of the previous approach. In my opinion, it may also be confusing. But at least here we have fewer buttons polluting the interface. That said though, maybe the yank button gets "invisible" because of crate owners being used to that area of the screen belonging to the follow button (both prior this PR and when viewing thirdy party crates). |
|
📌 Commit fca52d6 has been approved by |
|
☀️ Test successful - checks-travis |
…iagoarrais Temporarily remove yank button from crate header Temporarily remove the yank button from the crate page until we find better placement for the button. The buttons added to the all versions page in #2623 remain. I would like to merge this as a temporary fix to unblock deploying to production. I've opened #2794 to track adding this back. r? `@locks` cc `@thiagoarrais`
Trying to fix #1659
This adds a yank/unyank button to the crate and versions pages.
By the way, I'm learning Ember and the crates.io codebase for this one. Please review accordingly. I'm assuming that there is a lot of things out of place. Just let me know where everything should go.
Some questions:
I've based this button on the edit email button from the account page, but had to make it smaller because of lack of screen real estate. Is that ok? If so, should this live in the component's
yank-button.module.cssor should I create a.smallerentry inshared/buttons.module.css?Added
yank-button.jsjust to specify an empty tag name that in turn makes Ember avoid generating an enclosingdivthat breaks styling. Is there a cleaner way to do that?