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 a description field for zarf variable / constant declarations #844

Merged
merged 6 commits into from
Oct 6, 2022

Conversation

jeff-mccoy
Copy link
Contributor

No description provided.

src/config/variables.go Outdated Show resolved Hide resolved
@jeff-mccoy jeff-mccoy enabled auto-merge (squash) October 6, 2022 08:05
Copy link
Contributor

@Racer159 Racer159 left a comment

Choose a reason for hiding this comment

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

Approving for now, but I don't love the description above the prompt either (feels disconnected to me)... Tagging @Madeline-UX in case she has opinions we could address in a future PR (since it would be non-breaking visual changes). Would be nice to have a guide on the use of color, prompting and displaying info even for the CLI use case.

@jeff-mccoy jeff-mccoy merged commit 23f39d3 into master Oct 6, 2022
@Madeline-UX
Copy link
Contributor

Approving for now, but I don't love the description above the prompt either (feels disconnected to me)... Tagging @Madeline-UX in case she has opinions we could address in a future PR (since it would be non-breaking visual changes). Would be nice to have a guide on the use of color, prompting and displaying info even for the CLI use case.

@Racer159 Can you provide a screen capture of what you are describing here and how it looks in the CLI?

@jeff-mccoy
Copy link
Contributor Author

Yeah I'm not sure what he's referring to, this is consistent color/placement with all other prompts / descriptions fields used in zarf cli today. It would also be very difficult to actually code the text below the input field with the current library.

@Racer159
Copy link
Contributor

Racer159 commented Oct 7, 2022

@Madeline-UX @jeff-mccoy I may be thinking about this weirdly, but it just feels like the description is hanging out above the prompt. Part of that could also be the way the prompt/description is worded too, but usually I am used to having a title that goes before the description and prompt to ground it more.

This is how it is:
194244628-e932c363-b34e-4928-b16f-f969e3cbdcc5

This is closer to how I (subjectively) feel it should be:
image

Not a big deal hence my approval, but something that felt weird so I mentioned it.

@jeff-mccoy
Copy link
Contributor Author

I think you might be getting wrapped around the wording. We use this same format everywhere but the description field is not a question, that one just happened to be one in that example as a user can make it whatever they want. See component deployment prompts for how this works elsewhere.

@Racer159
Copy link
Contributor

Racer159 commented Oct 7, 2022

I think it is the bolding hierarchy too

@bdfinst bdfinst deleted the package-variable-description branch January 12, 2023 22:33
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.

3 participants