Skip to content

DM-11094: use default 'Time Series Tool' title #419

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

Merged
merged 1 commit into from
Jul 26, 2017
Merged

Conversation

tgoldina
Copy link
Contributor

@tgoldina tgoldina commented Jul 21, 2017

when appTitle is undefined or empty string

https://jira.lsstcorp.org/browse/DM-11094

To test, start Time Series Viewer from SUIT application:
LSST WISE Target: 9.6 -1.1, radius 100,
Check the title. It should not be empty or start with ':'

@tgoldina tgoldina requested a review from cwang2016 July 21, 2017 21:52
@@ -33,6 +33,7 @@ import {getAppOptions} from '../../core/AppDataCntlr.js';
import {HelpText} from './../../ui/HelpText.jsx';

const vFileKey = LC.FG_FILE_FINDER;
const DEFAULT_TITLE = 'Time Series Tool';
/**
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought this was set already in the entry point?

Copy link
Contributor Author

@tgoldina tgoldina Jul 21, 2017

Choose a reason for hiding this comment

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

It was set correctly when appTitle was undefined, but not when it was '', like in SUIT.
I did see defaults.appTitle = 'Time Series Tool' in IrsaViewer.js – if it's not used elsewhere, I would suggest removing it, since it's the default title anyway. @ejoliet what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

sure, i can remove it later.

@@ -115,12 +116,12 @@ export class LcViewer extends PureComponent {

};

let title = appTitle;
let title = appTitle ? appTitle : DEFAULT_TITLE; // use default title when appTitle is undefined or ''
Copy link
Contributor

Choose a reason for hiding this comment

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

or 'title = appTitle || DEFAULT_TITLE;'

@cwang2016
Copy link
Contributor

Review completes. It works.

@tgoldina tgoldina merged commit 533c768 into dev Jul 26, 2017
@tgoldina tgoldina deleted the DM-11094_apptitle branch July 26, 2017 17:25
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.

3 participants