This repository has been archived by the owner on Dec 5, 2022. It is now read-only.
Publications/Document container restyling #240
Conversation
This file contains 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
We do this throughout the website -- this isn't just limited to document collections/publications. There's a new fixture containing links to external URLs, internal pages, a 'pdf', 'docx' and random other file. (Really, the files are just the string "x" because we're only checking the extension -- they're not real so don't be surprised if you can't click through correctly.) To do: * Offsite link to image To not do right now: * remove Embed and Free Text components of the document group * counting the pages in a PDF.
https://designnotes.blog.gov.uk/2016/11/28/removing-the-external-link-icon-from-gov-uk/ suggests there's no clear need for it, and the current half-working implementation looks awful. We can add it back when we have more time.
jacksonj04
suggested changes
May 20, 2022
| self.assertEqual(title.text.strip(), "A Document With A Link To A Page") | ||
| self.assertEqual( | ||
| title.select_one("a")["href"], | ||
| "/blog-index-page/", | ||
| ) | ||
|
|
||
| def test_publication_pdf(self): | ||
| return # TODO: remove |
There was a problem hiding this comment.
This needs removing to reinstate this test path
| @@ -41,6 +41,22 @@ class DocumentBlock(StructBlock): | |||
| title = RichTextBlock(required=False) | |||
| document = DocumentChooserBlock() | |||
| summary = RichTextBlock(required=False) | |||
| extension_data = { | |||
There was a problem hiding this comment.
Nitpicky convention, since this is a constant
Suggested change
| extension_data = { | |
| EXTENSION_DATA = { |
| @@ -51,7 +67,9 @@ def get_context(self, value, parent_context): | |||
| context = super().get_context(value, parent_context) | |||
| context["file_ext"] = value["document"].file_extension | |||
| context["file_size"] = filesizeformat(value["document"].get_file_size()) | |||
|
|
|||
| context["ext_name"], context["ext_icon"] = self.extension_data.get( | |||
There was a problem hiding this comment.
Suggested change
| context["ext_name"], context["ext_icon"] = self.extension_data.get( | |
| context["ext_name"], context["ext_icon"] = self.EXTENSION_DATA.get( |
| self.assertIn(">File<", self.unknown) | ||
|
|
||
| def test_internal_page(self): | ||
| self.assertIn("link.svg", self.internal_page) # is this correct behaviour? |
There was a problem hiding this comment.
I think this is correct behaviour, but we should discourage this kind of thing in the operations manual.
| str(attachment) for attachment in soup.select(".attachment") | ||
| ] | ||
|
|
||
| def test_has_bytes(self): |
There was a problem hiding this comment.
It's not immediately clear what this is actually testing for, so it might be worth a clarifying comment.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
We've restyled the attachment block. There's a couple of fiddly bits that are handled specially:
tags for PDF documents to name the filetype in full
Handling for different SVGs for different filetypes, unknown filetypes and things that we just have a URL for (although the SVGs do look almost identical at the moment)
Possibly needs an accessibility pass for the alt text on the images.
Before example:
