Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Fix About Dialog Contributors (Issue 3012 from Pull 2934) #3014

Merged
merged 6 commits into from Mar 1, 2013
Merged

Fix About Dialog Contributors (Issue 3012 from Pull 2934) #3014

merged 6 commits into from Mar 1, 2013

Conversation

TuckerWhitehouse
Copy link
Contributor

This is a fix for #3012 in response to #2934 reworking the _getContributorsInformation function to return the $.getJSON call directly, as well as add a "graceful" fail message as suggested by @njx.

Hoping this could get merged into spring 21 rather than being pushed to 22.

Update the _getContributorsInformation function to return the $.getJSON call which is basically the deferred being returned anyway.

Add a "graceful" fail statement that hides the spinner and add a message for the user.
@@ -52,16 +52,7 @@ define(function (require, exports, module) {
* @return {$.Promise} jQuery Promise object that is resolved or rejected after the information is fetched.
*/
function _getContributorsInformation() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should just remove this function and replace _getContributorsInformation() and inside _handleAboutDialog() just call $.getJSON(brackets.config.contributors_url) as @njx proposed.

@TomMalbran
Copy link
Contributor

Great, but you still need to replace the text in .html with Strings.ABOUT_TEXT_LINE6 so that it can be seen in other languages too.

@ghost ghost assigned TomMalbran Mar 1, 2013
@njx
Copy link
Member

njx commented Mar 1, 2013

Cool, thanks for jumping on this @TuckerWhitehouse and @TomMalbran. Tom--if this looks good to you, feel free to merge now--we won't be closing the codebase for the sprint until some other stuff is merged anyway.

@TomMalbran
Copy link
Contributor

@njx Will do. I just have one last comment after checking how it looks when the request fails.

@TomMalbran
Copy link
Contributor

Thanks, awesome. I'll merge it now.

For the next time, you might want to consider using branches instead of pulling directly from your master, it helps a lot when the request takes longer to be merged, since you can just leave it there and work on other stuff on other branches or in the master.

TomMalbran added a commit that referenced this pull request Mar 1, 2013
Fix About Dialog Contributors (Issue 3012 from Pull 2934)
@TomMalbran TomMalbran merged commit 0e3fa80 into adobe:master Mar 1, 2013
@njx
Copy link
Member

njx commented Mar 1, 2013

Good catch on the font. (We should probably clean up the CSS at some point so we don't have to repeat that on every child of the parent.)

@TomMalbran
Copy link
Contributor

That should be easy enough to do in this case. But we might need to check every other template too.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants