Skip to content

feat: added resource section with article and books routes working #395

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

Conversation

Akshaybagai52
Copy link
Contributor

GitHub Issue: #282

Summary:

Do you think resolving this issue might require an Architectural Decision Record (ADR)? (significant or noteworthy)

Yes/No
[JUSTIFICATION]

Copy link
Collaborator

@benjagm benjagm left a comment

Choose a reason for hiding this comment

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

Please address this first wave of changes and we'll continue the review of UX when this is done.

.gitmodules Outdated
@@ -40,3 +40,6 @@
[submodule "_includes/dot-github"]
path = _includes/dot-github
url = https://github.com/json-schema-org/.github.git
[submodule "path/to/submodule"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should not import this data as submodule. We like to avoid this type of dependency from projects outside our org. We'll do our own version of the data file in our data folder.

className={classnames( 'ml-6', { 'hidden': !active.getResources })}
id='resources'
>
<DocLink uri='/resources/books' label='Books' />
Copy link
Collaborator

Choose a reason for hiding this comment

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

These are static links. I'd like to avoid this. Instead I'd like you to go through the data and extract the resource categories available there to build dynamically these links.

@@ -0,0 +1,80 @@
import React, { useEffect, useState } from "react";
Copy link
Collaborator

Choose a reason for hiding this comment

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

I expect one single generic resources page that accepts the resource type as parameters os it will work for all cases.

@@ -0,0 +1,76 @@
import React, { useEffect, useState } from "react";
Copy link
Collaborator

Choose a reason for hiding this comment

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

I expect one single generic resources page that accepts the resource type as parameters os it will work for all cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants