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

Add GraphQL output to the "Generate Intended Config" view #840

Merged
merged 2 commits into from
Dec 9, 2024

Conversation

gsnider2195
Copy link
Contributor

Screenshots

image

image

image

image

@gsnider2195 gsnider2195 requested a review from smk4664 December 4, 2024 00:20
Copy link
Contributor

@smk4664 smk4664 left a comment

Choose a reason for hiding this comment

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

I am starting my review with Safari to ensure that the CSS changes work with Safari.

Copy link
Contributor

@smk4664 smk4664 left a comment

Choose a reason for hiding this comment

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

Looks great! I tested in Firefox, Chrome and Safari and it looks good in all three!

@@ -23,7 +40,7 @@
<a href="{% url 'plugins:nautobot_golden_config:goldenconfig_list' %}">Config Overview</a> page.
</p>
<p>
This will render the configuration for the selected device using Jinja templates from the golden config <code>jinja_repository</code>
This will render the configuration for the selected device using Jinja templates from the Golden Config <code>jinja_repository</code>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This will render the configuration for the selected device using Jinja templates from the Golden Config <code>jinja_repository</code>
This will render the configuration for the selected device using Jinja templates from the Golden Config Setting <code>jinja_repository</code>

Copy link
Contributor

Choose a reason for hiding this comment

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

Though, the setting is just called Golden Config, so I could go either way on this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cmsirbu is going to try to get some input on the wording here

@@ -8,6 +8,23 @@
.button-container {
margin-bottom: 24px;
}
pre:has(code) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bootstrap has a default of 11.5px here so the code block inside of the pre block doesn't fill the box completely.

default

image

with this fix

image

@gsnider2195 gsnider2195 merged commit 49caca7 into develop Dec 9, 2024
15 checks passed
@gsnider2195 gsnider2195 deleted the u/gas-zh228-generate-intended-graphql branch December 9, 2024 22:24
@smk4664 smk4664 mentioned this pull request Dec 17, 2024
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.

4 participants