generated from maykinmedia/default-app
-
Notifications
You must be signed in to change notification settings - Fork 0
Port API project styles and templates #10
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
Merged
sergei-maertens
merged 11 commits into
main
from
feature/5-port-api-styles-and-templates
Jun 10, 2025
Merged
Port API project styles and templates #10
sergei-maertens
merged 11 commits into
main
from
feature/5-port-api-styles-and-templates
Jun 10, 2025
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2b49970
to
d8ca434
Compare
d8ca434
to
67a6c3c
Compare
This is a very pragmatic approach with some explicit assumptions about base templates and blocks being present, so it should be coordinated with the (base) templates in default project. For now the master.html template is not included at all, as this is something that often requires some tweaks in each project and we want to keep that flexibility.
67a6c3c
to
5538001
Compare
Made sure to make it extensible enough for the project-specific needs.
Decided not to use a CSS pre-processor and instead use native CSS nesting. The target audience for APIs are developers/technical people of which we can reasonably assume that they can install a browser newer than January 2024. The CSS itself is not that exciting that we expect large benefits from minifications, and it keeps our toolchain lean.
I'm sure there must also exist a pure CSS solution for this, something to explore for the future... For the time being, the minimal JS is included in the library and automatically loaded in the base template. Most of the API projects can now probably drop their (public facing) JS build toolchain.
5538001
to
5cf8bda
Compare
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
CharString
approved these changes
Jun 6, 2025
sergei-maertens
commented
Jun 6, 2025
…re for e2e tests * Made the test ignores depend on the select tox environment, just like how the OIDC enabled/disabled works in our django-digid-eherkenning package * Copied over the tox and pyproject config for playwright e2e tests, which will be used for the api landing page tabs interaction.
If you have all the extras installed locally for local development, running pytest would fail on the errors because it assumes it's running in the 'base' test environment in tox. Now, the tests are skipped in those situations, which adds a little bit more risk in being careful which extras you setup in tox.ini.
14bdf3b
to
cc10106
Compare
Added a simple template inheritance and other necessary bits to make the testapp look similar enough to a default-project project. The basic CSS is loaded and the tabs are interactive, this is now a good starting point to write some accessible playwright tests and improve the JS and markup accordingly.
Playwright sync API starts an event loop under the hood, and Django doesn't like that. I've initially tried to set this up as a module fixture, but then you still get warnings about Django/pytest failing to tear down the test database, stil because of the async context.
cc10106
to
61a10f9
Compare
CharString
requested changes
Jun 10, 2025
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.
Love it. Just one thing.
Added tests that demonstrate the correct accessible markup and keyboard event captures to make it possible to navigate the content tabs with a screenreader while providing proper accessible feedback about related elements.
All the credit goes to the MDN docs: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/tab_role The markup, JS and CSS are updated so that it's accessible and understood by screenreaders. Following Chris' feedback, the JS is updated to use accessible queries to select elements, which helps lock in the a11y requirements. The CSS is modified slightly to account for the (new) button element which comes with some default background styling out of the box, and the 'active' modifier is no longer required on the tab panels. Note that each tab element is now a button because that handles the Enter and spacebar keys properly to activate the associated tab panel.
61a10f9
to
842b8f9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #5
First iteration, tested in GPP-Woo/GPP-zoeken#92
The companion PR in default project is here: https://bitbucket.org/maykinmedia/default-project/pull-requests/153, I'll update/clean that up once this is merged and released.