-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Make UI styling consistent between libraries and courses tabs when empty (SOL-232) #7059
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -302,9 +302,9 @@ <h3 class="course-title">${course_info['display_name']}</h3> | |
| <div class="notice notice-incontext notice-instruction notice-instruction-nocourses list-notices courses-tab active"> | ||
| <div class="notice-item"> | ||
| <div class="msg"> | ||
| <h3 class="title">${_("Are you staff on an existing {studio_name} course?").format(studio_name=settings.STUDIO_SHORT_NAME)}</h3> | ||
| <h3 class="title">${_("Are you staff on an existing {studio_name} course?").format(studio_name=set)}</h3> | ||
| <div class="copy"> | ||
| <p>${_('You will need to be added to the course in {studio_name} by the course creator. Please get in touch with the course creator or administrator for the specific course you are helping to author.').format(studio_name=settings.STUDIO_SHORT_NAME)}</p> | ||
| <p>${_('The course creator must give you access to the course. Contact the course creator or administrator for the course you are helping to author.')}</p> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
@@ -443,11 +443,28 @@ <h3 class="course-title">${library_info['display_name']}</h3> | |
| <div class="notice notice-incontext notice-instruction notice-instruction-nocourses list-notices libraries-tab"> | ||
| <div class="notice-item"> | ||
| <div class="msg"> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @marcotuts @antoviaque @Kelketek Can you please clarify the workflow/use case here? I find it a bit strange that a user would end up at the empty library page and see BOTH the "request access from a library creator or admin" AND the "Create your First Library" messages. If a user does NOT have course author (= library creator) privileges, will they still see the "Create your First Library" button enabled? And if a user does have course author and library creator privileges then it seems confusing to present them with the first message.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The first message is prefixed with the title 'Expecting to see a library already here?' It's to inform users of the case when they may be in Studio and expect to be able to edit a library, only to find out there isn't one. This is the same behavior that happens for courses. The permissions check has not changed between this and the previous version-- @bradenmacdonald is there a permission type we should be checking for? I got the impression by the code that 'anyone can create a library', which may not be what we actually want, but if this is the case, the fix for it is probably outside the scope of this task.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, @Kelketek I had to check back in documentation for the main feature, to verify our position when we released the original MVP. In the documentation, we have: "In Studio, if you have course creation privileges, you can create a content library..." My understanding is that not all Studio users have permissions to create content libraries. Some might be course team members without course authoring privileges, and also shouldn't have library creation privileges, in which case the "Create First Library" button should ideally be hidden or disabled. @marcotuts your thoughts?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't believe we have different course creation privileges and library creation privileges, we just have creation privileges. @catong I can follow up in person in case I've missed anything, but I think the message is ok to have in either the library or course context, as authors may be expecting one or both of those to be possible.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @marcotuts Does this mean I should be checking for course authoring privileges when displaying the create button?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, @marcotuts Perhaps I'm still missing the point. My original question was about whether a library user should see BOTH of these things:
Are you saying that yes, both cases can apply simultaneously, because a course team member who is able to create their own library might not have access privileges to library that someone else created?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, wait... @marcotuts I think I still have an outstanding question. Per what we documented for the MVP and further to @Kelketek's question above, is my understanding correct that only users with course creation privileges also have library creation privileges? If yes, then there might be Studio users who do NOT have library creation privileges, in which case my question is still valid -- in the case of these users, I don't think they should see an active "Create a new library" button.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, there is a single "creator" privilege that affects the ability to create courses and libraries. If you look above in this file, you can see the button in the top right checks for
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bradenmacdonald thanks for shedding light! |
||
| <div class="copy"> | ||
| <p>${_("You don't have any content libraries yet.")}</p> | ||
| </div> | ||
| <h3 class="title">${_("Were you expecting to see a particular library here?")}</h3> | ||
| <div class="copy"> | ||
| <p>${_('The library creator must give you access to the library. Contact the library creator or administrator for the library you are helping to author.')}</p> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| %if course_creator_status == "granted": | ||
| <div class="notice-item has-actions"> | ||
| <div class="msg"> | ||
| <h3 class="title">${_('Create Your First Library')}</h3> | ||
| <div class="copy"> | ||
| <p>${_('Libraries hold a pool of components that can be re-used across multiple courses. Create your first library with the click of a button!')}</p> | ||
| </div> | ||
| </div> | ||
|
|
||
| <ul class="list-actions"> | ||
| <li class="action-item"> | ||
| <a href="#" class="action-primary action-create new-button action-create-library new-library-button"><i class="icon fa fa-plus icon-inline"></i> ${_('Create Your First Library')}</a> | ||
| </li> | ||
| </ul> | ||
| </div> | ||
| %endif | ||
| </div> | ||
| %endif | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was
settings.STUDIO_SHORT_NAMEchangedset. I assume this is a typo. This resulted in TNL-4167There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weird. Looks like a typo to me. Odd that nobody noticed it until a year later.