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

Added more flexibility for related content and combined related links #1314

Merged
merged 13 commits into from
Jan 26, 2021

Conversation

trevorsaint
Copy link
Contributor

@trevorsaint trevorsaint commented Jan 21, 2021

This component should be used in favour of related links.

  • Added more control and flexibility for related content
  • Added documentation and macro options

BREAKING CHANGE:

Removed the 'related links' component, to be replaced by 'Related content'.

Includes multiple row option

Screenshot 2021-01-21 at 10 04 51

Closes #1312

@codecov
Copy link

codecov bot commented Jan 21, 2021

Codecov Report

Merging #1314 (a2b117f) into master (ef32207) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1314   +/-   ##
=======================================
  Coverage   93.62%   93.62%           
=======================================
  Files          33       33           
  Lines        1506     1506           
=======================================
  Hits         1410     1410           
  Misses         96       96           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef32207...a2b117f. Read the comment docs.

@boxadesign
Copy link
Contributor

Looks good but all examples seem to have the census theme. Maybe good to have a default

@trevorsaint
Copy link
Contributor Author

Looks good but all examples seem to have the census theme. Maybe good to have a default

Ok @boxadesign I will get one added.

@trevorsaint
Copy link
Contributor Author

Copy link
Contributor

@jrbarnes9 jrbarnes9 left a comment

Choose a reason for hiding this comment

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

So can we remove 'related links' component now? It is possible only the Census website using it. Maybe leave it in as we don't know what other services are using is, but we should at least remove component from documentation, so no one uses it going forward. Perhaps put in release notes that this new macro should replace it? Then remove component macro later?

@trevorsaint
Copy link
Contributor Author

So can we remove 'related links' component now? It is possible only the Census website using it. Maybe leave it in as we don't know what other services are using is, but we should at least remove component from documentation, so no one uses it going forward. Perhaps put in release notes that this new macro should replace it? Then remove component macro later?

@jrbarnes9 I did want to remove the related links, but wanted to leave it in just in case it was being used elsewhere. In regards to release notes. Good idea. Both @boxadesign and @rmccar do the releases.

@trevorsaint trevorsaint requested a review from jrbarnes9 January 25, 2021 12:22
@trevorsaint trevorsaint marked this pull request as draft January 25, 2021 12:59
@trevorsaint trevorsaint added the Breaking changes This PR contains at least one breaking change label Jan 25, 2021
@trevorsaint trevorsaint marked this pull request as ready for review January 25, 2021 13:03
@trevorsaint trevorsaint requested a review from jrbarnes9 January 26, 2021 12:30
boxadesign
boxadesign previously approved these changes Jan 26, 2021
@trevorsaint trevorsaint merged commit 358ceb8 into master Jan 26, 2021
@trevorsaint trevorsaint deleted the 1312-related-content-enhancement branch January 26, 2021 14:54
boxadesign pushed a commit that referenced this pull request Feb 17, 2023
…#1314)

* Added more flexibility for related content. Added some documentation and guidance

* Added default examples for related content

* Removed related links component

* Updated markup to use aside instead of a div container

* Removed role attribute in favour of aria-label to provide understanding in purpose. Updated macro option as a required entry
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need to differentiate types of related content by sub headings
3 participants