Skip to content

IRSA-355: add extra style option to the LC viewer #353

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
Apr 20, 2017

Conversation

ejoliet
Copy link
Contributor

@ejoliet ejoliet commented Apr 14, 2017

This PR is adding additional style to the title so it can be layout on the right position of the icon.
Plus, the title will change depending on the page the user is. @ layouts have 2 different title.
See IRSA-355 for details.

Test: download both branches IFE and Firelfly related to IRSA-355. Build Firefly and IRSAViewer and check both LC viewer:

http://localhost:8080/firefly/ts.html (doesn't have icon, only title)
http://localhost:8080/irsaviewer/timeseries (have banner icon and title shifted)

@ejoliet ejoliet requested review from loitly and lznakano April 14, 2017 19:34
@lznakano
Copy link
Contributor

It looks good to me.

@lznakano lznakano requested review from lznakano and removed request for lznakano April 17, 2017 18:49
Copy link
Contributor

@lznakano lznakano left a comment

Choose a reason for hiding this comment

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

It looks good to me.

Copy link
Contributor

@loitly loitly left a comment

Choose a reason for hiding this comment

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

I would expect to see the Banner's title prop changed from string to an object, react element or something that allows you to send in whatever you want. But, this is fine.

@ejoliet ejoliet merged commit 57cf350 into rc Apr 20, 2017
@ejoliet ejoliet deleted the IRSA-355-add-main-lc-title branch April 20, 2017 07:29
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