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

'Document list' component to replace downloads and articles, and use for search results #2011

Merged
merged 34 commits into from
Mar 21, 2022

Conversation

jrbarnes9
Copy link
Contributor

@jrbarnes9 jrbarnes9 commented Feb 25, 2022

Contains breaking changes

What is the context of this PR?

Created a new single 'document list' component to be used to display similar types of document links/excerpts. Intended for:

  • downloadable documents (e.g. print resources)
  • news articles
  • search results

Each variant of this component contains:

  • Thumbnail image or placeholder (optional)
  • Document item title/link
  • Metadata (document type, date, file info)
  • Description (optional)
  • 'Featured' option, to draw attention to a single document displayed first on the page – aligned style to current 'featured' article

What are the breaking changes?

Developers using the following patterns and components will need to replace them with the new onsDocumentList() macro.

Removed components

The new 'document list' component replaces the now deprecated components:

  • articles
  • downloads

Affected patterns

The following patterns are updated to now use the new 'document' component in replace of the deprecated components listed above:

  • download resources (help users to...)
  • news (pages)

Closes #1797 and fixes #1664

How to review

Check macro, documentation, variants and examples of the components and patterns listed above.

@jrbarnes9 jrbarnes9 self-assigned this Mar 4, 2022
@jrbarnes9 jrbarnes9 linked an issue Mar 4, 2022 that may be closed by this pull request
@jrbarnes9 jrbarnes9 changed the title 1797 search result Document component to replace downloads and articles, and use for search results Mar 7, 2022
@jrbarnes9 jrbarnes9 marked this pull request as ready for review March 7, 2022 16:32
kruncher
kruncher previously approved these changes Mar 8, 2022
Copy link
Contributor

@kruncher kruncher left a comment

Choose a reason for hiding this comment

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

Looks good to me. The only thing that I wonder about is the name of the component; i.e. Document vs something like DocumentEntry or DocumentItem since the component presents information about a document rather than being the actual document.

@jrbarnes9 jrbarnes9 requested a review from kruncher March 10, 2022 09:19
@jrbarnes9 jrbarnes9 changed the title Document component to replace downloads and articles, and use for search results 'Document list' component to replace downloads and articles, and use for search results Mar 10, 2022
kruncher
kruncher previously approved these changes Mar 10, 2022
Copy link
Contributor

@rmccar rmccar left a comment

Choose a reason for hiding this comment

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

Just these few things, other than that looks good

src/components/document-list/_macro.njk Outdated Show resolved Hide resolved
src/components/document-list/_macro.njk Outdated Show resolved Hide resolved
src/components/document-list/_macro.njk Outdated Show resolved Hide resolved
kruncher
kruncher previously approved these changes Mar 10, 2022
jrbarnes9 and others added 2 commits March 11, 2022 10:45
Co-authored-by: rmccar <42928680+rmccar@users.noreply.github.com>
jrbarnes9 and others added 2 commits March 11, 2022 10:45
Co-authored-by: rmccar <42928680+rmccar@users.noreply.github.com>
@jrbarnes9 jrbarnes9 added the Do not merge Don't merge this PR until this label is removed label Mar 11, 2022
@boxadesign
Copy link
Contributor

image

Pagination layout issue on News pattern

@jrbarnes9
Copy link
Contributor Author

image

Pagination layout issue on News pattern

Yeah I raised a bug for this #2024

@jrbarnes9 jrbarnes9 merged commit 4bbcd73 into master Mar 21, 2022
@jrbarnes9 jrbarnes9 deleted the 1797-search-result branch March 21, 2022 10:42
@jrbarnes9 jrbarnes9 removed the Do not merge Don't merge this PR until this label is removed label Mar 22, 2022
boxadesign pushed a commit that referenced this pull request Feb 17, 2023
…for search results (#2011)

* added document component first draft

* refector

* added date

* added more metadata options

* corrected page template heading levels

* added examples of differenet document types

* added featured docs

* fixed issues with examples

* used new document component in news pattern

* documentation

* more documentation

* fixed bug #1664

* replaced download resources with new document component

* removed old components replaced by document

* removed unnecessary change

* removed unused document examples

* revert unrelated changes

* removed scss imports for removed components

* removed old placeholder thumbnail pngs

* removed unused imports from guide example

* revert removed onsList from guide rtl example

* macro options docs

* refactored download resources pattern to use document list

* fix for border and margin showing when preceded by hidden item

* revert test

* updated downloadable resources examples [test-visual]

* remove bottom border from inline featured doc

* changed name of component and fixed featured image size

* updated example folder names

* adjusted metadata line height

* pr suggestion

* Update src/components/document-list/_macro.njk

Co-authored-by: rmccar <42928680+rmccar@users.noreply.github.com>

* Update src/components/document-list/_macro.njk

Co-authored-by: rmccar <42928680+rmccar@users.noreply.github.com>

Co-authored-by: rmccar <42928680+rmccar@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking changes This PR contains at least one breaking change Component A component in the ONS Design System
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Search result item Download resources checkboxes cause console error
4 participants