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

Use an image for success #46

Closed
joejag opened this issue Jan 19, 2015 · 12 comments
Closed

Use an image for success #46

joejag opened this issue Jan 19, 2015 · 12 comments
Assignees
Milestone

Comments

@joejag
Copy link
Contributor

joejag commented Jan 19, 2015

If builds are all green. Allow the user to specify a URL for an image to load and displayed fullscreen.

@GentlemanHal
Copy link
Member

Which takes precedence, the success text or url?

@joejag
Copy link
Contributor Author

joejag commented Jan 21, 2015

Url, and we can visually disable the success text input box when a URL is
entered?
On 21 Jan 2015 12:43, "Christopher Martin" notifications@github.com wrote:

Which takes precedence, the success text or url?


Reply to this email directly or view it on GitHub
#46 (comment)
.

@GentlemanHal
Copy link
Member

I think ultimately this should be combined with #9 and we should just auto detect if it's a url or just plain text. We could make that change under issue #9, so to finish this issue I agree the url should just take precedence.

Although it might be easier to just explain that with some text on the page, given that tab is so sparse right now and we are potentially (if you agree) going to change it when we play #9.

@joejag
Copy link
Contributor Author

joejag commented Jan 21, 2015

There's an affinity between the two issues, but we can release this without completing #9 with too much rework right?

I like the idea of adding the textual description on the success page. That's something we need in general, and this one is a good place to start with.

Let's add text for completing this issue saying the URL takes precedence. Then on issue #9 let's write the text for what will appear on the screen before we start coding.

@cowley05
Copy link
Contributor

agree with all!

@cowley05
Copy link
Contributor

Ok so this is done to the best of my ability :s please QA this and see if its ok for next release...

@cowley05 cowley05 removed their assignment Jan 23, 2015
@GentlemanHal
Copy link
Member

Should we check the image is actually an image and can be loaded? The image I was testing with has disappeared and now I just get the broken image icon which looks crap.

@joejag
Copy link
Contributor Author

joejag commented Jan 26, 2015

We've got a preview that shows while they are configuring it.

A broken image logo when unavailable isn't too bad, I suppose we could say
that in text instead, but a broken image send that message.

What are our thoughts for doing something better?
On 26 Jan 2015 11:50, "Christopher Martin" notifications@github.com wrote:

Should we check the image is actually an image and can be loaded? The
image I was testing with has disappeared and now I just get the broken
image icon which looks crap.


Reply to this email directly or view it on GitHub
#46 (comment)
.

@GentlemanHal
Copy link
Member

Can't really think of anything other than manually making a call to the url and checking the status code and content type.

I agree for this issue we shouldn't worry but it might be worth looking into for the future as showing a full screen broken image for success would make Nevergreen look bad (even though the user has selected a broken link)

We could try doing something nicer in issue #9, as that would allow us to just skip showing broken links.

@cowley05
Copy link
Contributor

Think this is pretty much there but needs @mrgeorgegray to sort out the scalling issue if you use funny sized images. I think as it is though we could release it like this and put another issue in next release or so to fix it?

@GentlemanHal
Copy link
Member

I agree this issue can be closed and we can open another if we decide the scaling issue is worth fixing.

@cowley05
Copy link
Contributor

think i may have fixed it :)

cowley05 added a commit that referenced this issue Jan 27, 2015
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

No branches or pull requests

3 participants