Skip to content
This repository has been archived by the owner on Nov 28, 2022. It is now read-only.

Added upgrade notification component to About page. #102

Merged
merged 1 commit into from Jul 15, 2016
Merged

Added upgrade notification component to About page. #102

merged 1 commit into from Jul 15, 2016

Conversation

vkandy
Copy link
Contributor

@vkandy vkandy commented Jun 30, 2016

Closes "Move the "update available" notification to the about page." part of #5071

Currently when a new upgrade is available, a banner displayed on top of the header, on all pages, which doesn't go away until the blog is upgraded. This message was moved to About page. The message won't go away till the blog is upgraded but it's far less annoying.

  • New release notifications are shown only in About page instead of as a banner on the top
  • Added gh-upgrade-notification component and upgrade-notification service
  • app/routes/application.js looks for notifications of type "upgrade" and invokes upgrade-notification service to render the message in about.hbs.
  • Added gh-upgrade-notification CSS classes to render message in red.
  • Picked some fixes https://github.com/TryGhost/Ghost/pull/5670/files

@@ -0,0 +1,8 @@
import Ember from 'ember';

This comment was marked as abuse.

@acburdine
Copy link
Member

WRT the ember destructuring - most if not all of the files in the ember codebase have been converted to use it so there's lots of examples 😄

Congrats on the first PR though, looks good 👍

@vkandy
Copy link
Contributor Author

vkandy commented Jul 1, 2016

@acburdine thanks a bunch for the tip! I've fixed destructuring and linting errors. Hope this works.

@vkandy vkandy changed the title [WIP] Added upgrade notification component to About page. Added upgrade notification component to About page. Jul 5, 2016
@acburdine
Copy link
Member

@vkandy Apologies if I wasn't clear in my initial comment. This is what I meant by destructuring:

instead of this:

import Ember from 'ember';

const {
  Component
  inject: {service}
} = Ember;

export default Component.extend();

Do this:

import Component from 'ember-component';
import injectService from 'ember-service/inject';

export default Component.extend();

This uses the shims from this library and there's a couple example PR's that I did recently that convert a lot of the const destructuring over to the new shims imports: #95 #101 #110 .

Sorry if it seems a bit nitpicky, but since most if not all of the code uses the new shims code it'd be nice to have it consistent 😄

@vkandy
Copy link
Contributor Author

vkandy commented Jul 15, 2016

This has fallen behind but I've fixed the tests and grunt validate and grunt test-ember look OK. Please review. Thanks!

@acburdine
Copy link
Member

hey @vkandy I'll review this in a bit, but would you mind rebasing against master so it's easier to tell what your changes are? right now it includes changes from several other pull requests.

Easiest way to do that:

git checkout master
git pull upstream master
git checkout feature/5670-update-notifications-about-page
git rebase master
git push -f origin feature/5670-update-notifications-about-page

more help here: https://github.com/TryGhost/Ghost/wiki/Git-workflow

- Picked some fixes https://github.com/TryGhost/Ghost/pull/5670/files
- Destructured Ember properties
- Removed unused imports and fixed unit test errors.
@vkandy
Copy link
Contributor Author

vkandy commented Jul 15, 2016

@acburdine I fixed the commit history. Hope this is OK? Thanks for all the tips and being patient!

@acburdine
Copy link
Member

Looks great, thanks! Will review this later today hopefully.

@acburdine
Copy link
Member

LGTM 👍

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