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

Improvement: Add a direct 'view course' icon on the course management pages, solves #129. #417

Merged
merged 1 commit into from
Nov 4, 2023

Conversation

abias
Copy link
Member

@abias abias commented Oct 9, 2023

No description provided.

@lucaboesch lucaboesch added the feature Something which is a new feature or big improvement label Oct 10, 2023
@lucaboesch lucaboesch self-requested a review October 10, 2023 13:26
Copy link
Collaborator

@lucaboesch lucaboesch left a comment

Choose a reason for hiding this comment

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

Oh my, I'm sorry to rain on the parade. It's probably also due to time we let pass to develop this. But I just noticed we call that 'thing' all sort of names: "link", "icon", "button".
That's no good.
We should stick to something and then call it the same way throughout everything (also renaming the Issue title and Pull request title).
Now when there would also be a possibility to add the 'thing' "action", I would propose to go for 'icon'.
So the Site administration setting would become '"Show view course icon" theme_boost_union | showviewcourseiconincoursemgnt'
Also in the descriptive text there could be mentioned the other two-click way to go to course view "By default, on the course management page, Moodle requires you to open the course details before you can view the course. By enabling this setting, you can add a 'View course' icon directly to the category listing on the course management page."

"By default, on the course management page, Moodle requires you to either open the course details or to pass through the course settings before you can click an additional UI element to view the course. By enabling this setting, you can add a 'View course' icon directly to the category listing on the course management page."

Very nit-picky but probably worth it.

@lucaboesch
Copy link
Collaborator

lucaboesch commented Oct 10, 2023

Also, what about trying to use the course icon (pretend it's the link color)?

 issue-129

@whuml whuml linked an issue Oct 19, 2023 that may be closed by this pull request
@abias abias removed the feature Something which is a new feature or big improvement label Nov 2, 2023
@abias abias changed the title Improvement: Add a direct 'view course' link on the course management pages, solves #129. Improvement: Add a direct 'view course' icon on the course management pages, solves #129. Nov 3, 2023
@abias
Copy link
Member Author

abias commented Nov 3, 2023

Hey @lucaboesch ,

thank you very much for your insights!
I fully agree that we should use a consistent terminology. I have changed the patch to use "icon" consistently.

Regarding your proposal about the icon, I think I can't change this icon and the mortarboard icon is perfectly fine.
If you look at the patch, you will see that it just uses the pix_icon i/course. And this icon is mapped to fa-graduation-cap on https://github.com/moodle/moodle/blob/master/lib/classes/output/icon_system_fontawesome.php#L217. The three-dots-circle icon is an invention of previous Moodle versions and simply does not exist in font awesome.

I have pushed an updated patch and would then merge the PR as soon as the tests have passed again.

Cheers,
Alex

@abias abias requested a review from lucaboesch November 3, 2023 21:40
@abias abias merged commit 4affa26 into master Nov 4, 2023
12 checks passed
@abias abias deleted the issue-129 branch November 4, 2023 06:07
lucaboesch pushed a commit to lucaboesch/moodle-theme_boost_union that referenced this pull request Nov 5, 2023
abias added a commit that referenced this pull request Nov 12, 2023
abias added a commit that referenced this pull request Nov 12, 2023
abias added a commit that referenced this pull request Nov 12, 2023
detomon pushed a commit to detomon/moodle-theme_boost_union that referenced this pull request Aug 26, 2024
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.

Feature: Improve usability of course management page
2 participants