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

When no zone name is available display a default #1477

Merged
merged 1 commit into from
May 11, 2016

Conversation

remibergsma
Copy link
Contributor

When a zone name is available, the previous behaviour is still there:
screen shot 2016-03-30 at 21 00 44

When there is no zone name, it used to display an empty name (where you had to click on to see details):
screen shot 2016-03-30 at 21 03 06

With this change, a default name All is displayed (because this happens when S3 storage is used that is region wide aka all zones):
screen shot 2016-03-30 at 20 53 50

Region wide S3:
screen shot 2016-03-30 at 21 04 55

FYI: Screenshot shows 'All', later renamed to 'All Zones', see code.

@pdube
Copy link
Contributor

pdube commented Apr 23, 2016

LGTM. Do you think we should translate this?

@rohityadavcloud
Copy link
Member

LGTM

A manual UI testing is needed

tag:mergeready

@swill
Copy link
Contributor

swill commented May 2, 2016

I think this is ready to merge. @remibergsma has shown screenshots to verify the UI change does what it is supposed to and we have the LGTMs required.

@rohityadavcloud
Copy link
Member

@swill apart from the PR author, it would be great if at least one more reviewer can share a screenshot; please proceed as per your discretion

@swill
Copy link
Contributor

swill commented May 2, 2016

@rhtyd in general, I agree with you. The main blocker with that is the fact that very few people have the ability to test regional templates. The code is pretty straight forward too, so that helps our confidence in the change...

@pdube do you have the ability to test this PR with our swift setup to verify it works as expected?

@swill
Copy link
Contributor

swill commented May 6, 2016

Unless someone is able to verify this, I think we have to take the screenshots from @remibergsma as verification. Not very many people have the ability to test regional templates, so we have a very limited group of users who can give us verification on this. If we don't have any verification by Monday, I will likely merge this and count on @remibergsma's verification as proof that it works...

@swill
Copy link
Contributor

swill commented May 11, 2016

I am going to merge this one. I am confident with the change and I have verification from @remibergsma, so I think we are good to go on this one.

@asfgit asfgit merged commit aead9f4 into apache:4.7 May 11, 2016
asfgit pushed a commit that referenced this pull request May 11, 2016
When no zone name is available display a defaultWhen a zone name is available, the previous behaviour is still there:
![screen shot 2016-03-30 at 21 00 44](https://cloud.githubusercontent.com/assets/1630096/14154026/ba41a4bc-f6ba-11e5-9f88-19cf36bfbd4f.png)

When there is no zone name, it used to display an empty name (where you had to click on to see details):
![screen shot 2016-03-30 at 21 03 06](https://cloud.githubusercontent.com/assets/1630096/14154048/d31a7b08-f6ba-11e5-9f67-f716e8d9fbf2.png)

With this change, a default name `All` is displayed (because this happens when S3 storage is used that is region wide aka all zones):
![screen shot 2016-03-30 at 20 53 50](https://cloud.githubusercontent.com/assets/1630096/14154060/e20d1d0a-f6ba-11e5-9b0a-1b5e502a2964.png)

Region wide S3:
![screen shot 2016-03-30 at 21 04 55](https://cloud.githubusercontent.com/assets/1630096/14154108/2222ff54-f6bb-11e5-845a-c22ddc745b98.png)

FYI: Screenshot shows 'All', later renamed to 'All Zones', see code.

* pr/1477:
  When no zone name is available display a default

Signed-off-by: Will Stevens <[email protected]>
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.

5 participants