-
Notifications
You must be signed in to change notification settings - Fork 6
Reservation page shortcuts #787
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #787 +/- ##
=======================================
Coverage 88.01% 88.01%
=======================================
Files 153 153
Lines 6224 6224
=======================================
Hits 5478 5478
Misses 746 746 🚀 New features to boost your workflow:
|
TheStrgamer
left a comment
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.
Works when i run it on normal and mobile view, and couldn't find any bugs. LGTM
ddabble
left a comment
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.
Nice, code looks good overall 😊
| color: black !important; | ||
| } | ||
|
|
||
| @media only screen and (max-width: 700px) { |
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.
Should ideally stick to the pixel values used elsewhere on the website (e.g. these in style.css), for a consistent design :)
| </div> | ||
|
|
||
| <div class="reservation-machine-shortcut-buttons"> | ||
| <h4>{% translate "Shortcuts:" %}</h4> |
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.
An <h4> tag on its own can be jarring for people with screen readers when there is no <h3> (and <h2> and <h1>) tag that it's logically structured under.
So I think we should either update the other parts of the code to include those <h3>, <h2> and <h1> tags (instead the existing <div>s) - this is probably out of scope for this PR - or we should change this <h4> to a <div> with a class that's styled with a larger font size and top and bottom margins.
| #~ msgid "You have completed the Raise3D printer course" | ||
| #~ msgstr "Du har fullført Raise3D-printerkurset" |
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.
Nice removing these 👍
| </div> | ||
|
|
||
| <div class="reservation-machine-shortcut-buttons"> | ||
| <h4>{% translate "Shortcuts:" %}</h4> |
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.
Good to avoid having these types of trailing punctuation as part of the translation message, as then the message can be reused elsewhere without that punctuation :)
| <h4>{% translate "Shortcuts:" %}</h4> | |
| <h4>{% translate "Shortcuts" %}:</h4> |
|
|
||
| ### Improvements | ||
|
|
||
| - Added shortcuts to machine page, to help users navigate |
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.
Should add a link to the PR, like the other entries below:
| - Added shortcuts to machine page, to help users navigate | |
| - Added shortcuts to machine page, to help users navigate (MAKENTNU/web#787) |
Proposed changes
Added shortcut buttons to the reservations page, making it possible to jump directly to the chosen equipment type (suggested by Asta)
Checklist
(If any of the points are not relevant, mark them as checked, so that it's easy to see which points you've handled or not)
makemigrations,makemessagesandcompilemessages