Skip to content

Individual release view #2879

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

Merged
merged 19 commits into from
Feb 9, 2018
Merged

Individual release view #2879

merged 19 commits into from
Feb 9, 2018

Conversation

nlhkabu
Copy link
Contributor

@nlhkabu nlhkabu commented Feb 2, 2018

Fixes #424, fixes #2807, fixes #2808.

screenshot from 2018-02-02 18-08-08

screenshot from 2018-02-02 18-02-06

TODO:

  • Make link work when user clicks on 'collaborators' from this page (currently returning a 404)
  • Determine if the table is showing the most important information
  • Fix the layout on mobile/tablet
  • Hook up 'delete release' modal
  • Hook up 'delete file' modal. Maybe make this smarter, rather than using a loop?
  • Make 'copy checksum' add a 'copied' message at the top of the page
  • Add 'delete all files' shortcut (?)

@nlhkabu
Copy link
Contributor Author

nlhkabu commented Feb 2, 2018

hi @di @dstufft @ewdurbin - I need some feedback on what information is most useful to display in the table here. It is all needed/used? For example, what is the purpose of the comments? Thanks.

@di
Copy link
Member

di commented Feb 2, 2018

@nlhkabu I think everything you have in the table should probably stay.

I'm not really sure how users are using the "Comment" field for a release, it's optional and there's no limit on length, so it's possible it could be quite long but in my experience it's mostly unused. Maybe we could remove the column and add some kind of a sub-row if the release has a comment?

Add 'delete all files' shortcut (?)

Not sure this is necessary, a user will probably only want to delete a single file, or an entire release.

@di di force-pushed the individual-release-view branch 2 times, most recently from f58d986 to f9ee018 Compare February 2, 2018 22:53
@di
Copy link
Member

di commented Feb 2, 2018

@nlhkabu I fixed the collaboration 404 and hooked up the 'delete release' modal. I'm seeing an issue where the modal doesn't dismiss when there is an error, could you take a look?

Still need to hook up individual file deletion.

@di di added this to the 1: Maintainer MVP milestone Feb 5, 2018
@di di force-pushed the individual-release-view branch from f9ee018 to a39a9da Compare February 6, 2018 01:01
@di
Copy link
Member

di commented Feb 6, 2018

@nlhkabu I hooked up individual file deletion as well, back to you!

@nlhkabu
Copy link
Contributor Author

nlhkabu commented Feb 6, 2018

I've added the 'comment' field as a tooltip on the main table. The content will truncate at 60 chars.

screenshot from 2018-02-06 07-37-26

Responsive now working. I've made the decision here to hide type and version on smaller screens (for lack of space):

screenshot from 2018-02-06 20-06-48

screenshot from 2018-02-06 20-07-02

screenshot from 2018-02-06 20-07-12

screenshot from 2018-02-06 20-07-23

screenshot from 2018-02-06 20-07-32

@nlhkabu
Copy link
Contributor Author

nlhkabu commented Feb 6, 2018

fyi @di - I've again made the error messages more explicit, e.g.

screenshot from 2018-02-06 20-08-47

I will look into why the modals are staying open now :)

@di
Copy link
Member

di commented Feb 6, 2018

fyi @di - I've again made the error messages more explicit

Oops, did I merge over this? Apologies if I did!

@@ -117,7 +117,7 @@ <h3>Delete Release</h3>

<div id="delete-release-modal" class="modal">
<div class="modal__content" role="dialog">
<form method="POST" class="modal__form">
<form method="POST" class="modal__form" action="{{ request.route_path('manage.project.release', project_name=project.name, version=release.version) }}">
Copy link
Member

Choose a reason for hiding this comment

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

Aha! This makes sense. We can use action="{{ request.current_route_path() }}" here instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated, thanks :)

@nlhkabu nlhkabu force-pushed the individual-release-view branch from 3879df2 to 09142ac Compare February 7, 2018 06:39
@nlhkabu
Copy link
Contributor Author

nlhkabu commented Feb 9, 2018

Checksums modal:

screenshot from 2018-02-09 07-32-49

screenshot from 2018-02-09 07-33-14

screenshot from 2018-02-09 07-33-26

@nlhkabu nlhkabu force-pushed the individual-release-view branch 4 times, most recently from 30c3e34 to e2184ef Compare February 9, 2018 07:50
@nlhkabu
Copy link
Contributor Author

nlhkabu commented Feb 9, 2018

Hi @di - this should be ready for final review and merge :)

@nlhkabu nlhkabu force-pushed the individual-release-view branch from e2184ef to 0a4a649 Compare February 9, 2018 07:51
@di di force-pushed the individual-release-view branch from 0a4a649 to 15c018a Compare February 9, 2018 16:10
@di di changed the title WIP: Individual release view Individual release view Feb 9, 2018
@di di merged commit bf0495b into master Feb 9, 2018
@di di deleted the individual-release-view branch February 9, 2018 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants