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

DOCS-2361: Autogenerated Felix docs #1717

Merged
merged 1 commit into from
Oct 22, 2024
Merged

Conversation

ctauchen
Copy link
Collaborator

@ctauchen ctauchen commented Oct 11, 2024

Together with projectcalico/calico#9325, this PR introduces major changes to how our Felix configuration docs are presented.

Major items:

  • All data comes from the codebase

  • Diffs between codebase and docs are eliminated (plus cross-version, cross-product diffs)

  • Process much easier to maintain going forward

  • Presentation for users is VASTLY improved.

  • Felix CRD page

  • Felix config/env page

Product Version(s):

Issue:

Link to docs preview:

SME review:

  • An SME has approved this change.

DOCS review:

  • A member of the docs team has approved this change.

Additional information:

Merge checklist:

  • Deploy preview inspected wherever changes were made
  • Build completed successfully
  • Test have passed

@ctauchen ctauchen added the WIP label Oct 11, 2024
@ctauchen ctauchen requested a review from a team as a code owner October 11, 2024 11:51
Copy link

netlify bot commented Oct 11, 2024

Deploy Preview for calico-docs-preview-next ready!

Name Link
🔨 Latest commit 1e1b852
🔍 Latest deploy log https://app.netlify.com/sites/calico-docs-preview-next/deploys/6717bd9ae534a70008d2b06f
😎 Deploy Preview https://deploy-preview-1717--calico-docs-preview-next.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 44 (🔴 down 11 from production)
Accessibility: 90 (no change from production)
Best Practices: 83 (no change from production)
SEO: 92 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Oct 11, 2024

Deploy Preview succeeded!

Built without sensitive environment variables

Name Link
🔨 Latest commit 1e1b852
🔍 Latest deploy log https://app.netlify.com/sites/tigera/deploys/6717bd9aac87f90008ca2778
😎 Deploy Preview https://deploy-preview-1717--tigera.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 45 (🔴 down 2 from production)
Accessibility: 90 (no change from production)
Best Practices: 75 (no change from production)
SEO: 93 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@ctauchen ctauchen force-pushed the felix-stuff branch 3 times, most recently from 13b8786 to ebd3f82 Compare October 16, 2024 16:03
</table>
);

const TableEnv = ({ fieldData }) => (
Copy link
Collaborator

Choose a reason for hiding this comment

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

For the sake of neatness, you could extract these components into their own files and import them

/FelixConfig
  - index.js
  - file.json
  - TableEnv.js
  - TableConfig.js
  - TableCRD.js

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

}

return (
<div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

no need for the empty div (unless you actually need it). You can do

return <>{table}</>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, will do.

return <p>No matching group found for '{name}'.</p>;
}


Copy link
Collaborator

Choose a reason for hiding this comment

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

make sure you format all of your files option + shift + f. You may need to install prettier if it doesn't work

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done, thanks!

</tr>
<tr>
<td>Description</td>
<td dangerouslySetInnerHTML={{ __html: fieldData.DescriptionHTML }} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should probably sanitize this html to be safe
https://blog.logrocket.com/using-dangerouslysetinnerhtml-react-application/

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@ctauchen ctauchen changed the title Autogenerated Felix docs testing DOCS-2361: Autogenerated Felix docs Oct 17, 2024
@ctauchen ctauchen force-pushed the felix-stuff branch 2 times, most recently from fabc45c to ba04c2a Compare October 17, 2024 15:27
<div>
{matchedGroup.Fields.map((field, index) => (
<div key={index}>
<Tabs groupId='operating-systems'>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I noticed all tabs would change at the same time. Not sure if you intended this, but to stop that behaviour remove groupId='operating-systems'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That bit was intentional.

<div key={index}>
<Tabs groupId='operating-systems'>
<TabItem
value='apple'
Copy link
Collaborator

Choose a reason for hiding this comment

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

you should probably replace value='apple/orange' with something more meaningful like value='configurationFile/environmentVariable'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes!

@ctauchen ctauchen force-pushed the felix-stuff branch 2 times, most recently from b48d655 to f730d7f Compare October 21, 2024 14:46
@ctauchen ctauchen removed the WIP label Oct 21, 2024
@ctauchen ctauchen force-pushed the felix-stuff branch 2 times, most recently from 6565a82 to 93e925e Compare October 21, 2024 16:12
@ctauchen ctauchen merged commit 5254662 into tigera:main Oct 22, 2024
9 of 10 checks passed
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