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

Make attachment upload more understandable, indicate if attachment unreleased #1607

Merged
merged 4 commits into from
Sep 26, 2022

Conversation

20wildmanj
Copy link
Contributor

@20wildmanj 20wildmanj commented Sep 24, 2022

Previously, attachment upload was very confusing, with the buttons not being helpful / informative. Also, if an attachment was unreleased, the instructor wouldn't know unless they went into the edit page for the attachment. I tried to make the upload form give more informative information for each field, and added a (Not Released) to any attachments not released when looking at the assessment page.

Before:
(Earfs is unreleased, but you would never know...)
Screen Shot 2022-09-24 at 15 11 12
Screen Shot 2022-09-24 at 15 11 23

After:
Still kinda ugly, could polish if that is desired:
Screen Shot 2022-09-24 at 15 08 32
New attachment:
Screen Shot 2022-09-24 at 15 17 05
Edit attachment:
Screen Shot 2022-09-24 at 15 16 49

@damianhxy
Copy link
Member

I think the italics for (Not Released) is unnecessary. Also it seems that you swapped the screenshots for New attachment and Edit attachment.

<%= f.text_field :mime_type,
help_text: "Note: If you change the Mime type of a file, you might need to clear your browser's cache in order for you to see the change." %>
<% end %>
<ul>
Copy link
Member

Choose a reason for hiding this comment

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

Is the purpose of <ul> to make the lines the same length?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pretty much

Copy link
Member

@damianhxy damianhxy left a comment

Choose a reason for hiding this comment

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

Left some comments, but otherwise lgtm!

@20wildmanj 20wildmanj merged commit 52b072f into master Sep 26, 2022
@20wildmanj 20wildmanj deleted the assessment-attachment branch September 26, 2022 04:14
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