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 installation values to AppView #1060

Merged
merged 4 commits into from
Jun 25, 2019

Conversation

andresmgot
Copy link
Contributor

Fixes #739

Add the installation values used for the current release

Screenshot from 2019-06-19 12-23-06

@prydonius
Copy link
Contributor

Is there a discussion about the design of this? Just wondering why we chose to place this at the bottom of the AppView and not have a button or something that opens up a modal so it's less distracting?

@andresmgot
Copy link
Contributor Author

Is there a discussion about the design of this? Just wondering why we chose to place this at the bottom of the AppView and not have a button or something that opens up a modal so it's less distracting?

There is not, this is just my proposal. I thought about that but since there is nothing below the installation values it should not really bother users. IMO there is no reason to "hide" it and make it a modal but I am open to suggestions.

@andresmgot
Copy link
Contributor Author

@Angelmmiguel what do you think?

Copy link
Contributor

@prydonius prydonius left a comment

Choose a reason for hiding this comment

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

Thanks for the explanation, I'm okay with merging this with it displayed here for now, though we may want to reconsider this when we approach the redesign

@Angelmmiguel
Copy link
Contributor

@Angelmmiguel what do you think?

I don't have a strong opinion of this one because it depends on the relevance of this information. If users are constantly looking for it I believe that showing it is the best approach. On the other hand, if it's not that important, we should hide it and show it "on demand".

Thanks for the explanation, I'm okay with merging this with it displayed here for now, though we may want to reconsider this when we approach the redesign

Agree with @prydonius :)

@andresmgot
Copy link
Contributor Author

Thank you both for the input!

@andresmgot andresmgot merged commit be8b682 into vmware-tanzu:master Jun 25, 2019
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.

View values file used for application release
3 participants