Skip to content

Adding courseinfo to QR Code screen and devcontainer build setup#900

Open
mgmodell wants to merge 1 commit into
danmarsden:MOODLE_500_STABLEfrom
mgmodell:MOODLE_500_STABLE
Open

Adding courseinfo to QR Code screen and devcontainer build setup#900
mgmodell wants to merge 1 commit into
danmarsden:MOODLE_500_STABLEfrom
mgmodell:MOODLE_500_STABLE

Conversation

@mgmodell

Copy link
Copy Markdown

This should satisfy #857 .
Full disclosure, I used copilot to help as I was struggling to get the dev environment set up. Therefore, in addition to the small addition to the QR Code screen, we also have. devcontainer setup that's been debugged and verified to work on my M1 mac using vscode. If you have the extension installed, it will build the moodle environment and launch it upon 'reopen in dev container.' There's also a script in the /scripts folder to create a course with a few students and an attendance. That way I was able to launch and test everything pretty quickly.

I fiddled a bit with the layout to find something that I like, but it might break some style rules and it's easy enough to change if needed. I suspect most people won't like it as is. Happy to help if so, but this is at least functional.

Finally, not sure the policy on version bumps, but I tried to update it appropriately.

@danmarsden

Copy link
Copy Markdown
Owner

Thanks @mgmodell - best to pull out all the devcontainer stuff from the PR if you would like me to consider this - I don't think I've seen that in any moodle plugins and it's not something I'm currently looking at doing.

Also - I'm not quite sure if simply adding header tags is enough to make it display well - might be worth you dropping in a screenshot of how it looks into the PR as well?

Once that's done, please squash your commits into a single commit - I'm also a strong advocate for people using git commit trailiers when their code has been created with the use of AI tooling. - for example adding something like the following to the end of your commit message:
Ai-assisted-by: Copilot

thanks!

@mgmodell mgmodell marked this pull request as draft April 22, 2026 11:54
@mgmodell

Copy link
Copy Markdown
Author

Understood @danmarsden .

Here's what the current revisions look like. I'm happy to make changes, just let me know what you'd prefer. The page is already pretty simple and this gives me what I needed from it, but I was aiming for functional since I wasn't sure where to look for a design/style guide. Is there one I ought to consult?

I generally do not show the password as I use the QR with it embedded:
QR - no password

This is what it looks like with the password on screen:
QR with password

@danmarsden

Copy link
Copy Markdown
Owner

thanks for adding the screenshot - I think it might look better to remove the "course:" and "Session:" text - just display the course fullname, and then underneath that show the session date. (no need for the session date to show on both lines either) - but other than that - using the header tags looks sufficient for now.

@mgmodell

Copy link
Copy Markdown
Author

Gotcha!
So, actually, the script threw the date onto the attendance name to make it distinct - it wasn't built in or anything. Below is a revised version which gets rid of the labels and also uses a revised name without the date. Is this good? Shall we go with it?
Revised QR (no password)

@danmarsden

Copy link
Copy Markdown
Owner

yes - that looks better to me - feel free to send through a new PR with the other changes requested and I'll try to review when I get some spare time.

@mgmodell mgmodell force-pushed the MOODLE_500_STABLE branch from 9ab3957 to 6a37d0c Compare April 30, 2026 00:53
@mgmodell mgmodell mentioned this pull request Apr 30, 2026
@mgmodell

Copy link
Copy Markdown
Author

I think this meets your request, @danmarsden , but please let me know if it does not.
I did not include tests because I was struggling to get any tests to run. I really don't know phpunit or behat or even composer. It's been ages since I've touched php at all, to be honest.

@danmarsden

Copy link
Copy Markdown
Owner

yeah - pretty close - if you look at the "files changed" tab here in github we only want to see files changed that are part of this fix. you patch inscludes changes to .gitignore, composer.json and composer.lock that shouldn't be there.

I'd also be inclined to stop using lang strings and just inject the course fullname and sessiondate eg:
echo html_writer::tag('h2', userdate($session->sessdate));

also - when printing $course->fullname we need to pass it through format_string in case it has any multi-lang tags etc.
eg:
echo html_writer::tag('h3', format_string($course->fullname, true, array('context' => $context)));

@mgmodell mgmodell force-pushed the MOODLE_500_STABLE branch from 7c6d8e3 to c4b5754 Compare May 20, 2026 11:57
@mgmodell

Copy link
Copy Markdown
Author

How's this, @danmarsden ?

@danmarsden danmarsden marked this pull request as ready for review May 21, 2026 07:43
@danmarsden

Copy link
Copy Markdown
Owner

yeah - looks good - I'm just not sure why it hasn't triggered the github actions to run codechecker etc in the PR... There's a small conflict in the version.php file but I can fix that manually - just need to work out why the actions aren't being triggered.

@danmarsden danmarsden changed the base branch from MOODLE_500_STABLE to MOODLE_501_STABLE May 21, 2026 07:46
@danmarsden danmarsden changed the base branch from MOODLE_501_STABLE to MOODLE_500_STABLE May 21, 2026 07:46
@mgmodell

Copy link
Copy Markdown
Author

Fantastic!
Any insight into when I might expect this to be released? Specifically, when my students will be able to benefit from it?

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