Skip to content

Dm 5361 imgselpan#35

Merged
cwang2016 merged 5 commits intodevfrom
DM-5361-imgselpan
Mar 16, 2016
Merged

Dm 5361 imgselpan#35
cwang2016 merged 5 commits intodevfrom
DM-5361-imgselpan

Conversation

@cwang2016
Copy link
Copy Markdown
Contributor

More updates will be added after tab is made as one member in the field group.

@robyww
Copy link
Copy Markdown
Contributor

robyww commented Mar 15, 2016

UI Notes:

  • Take out 3 color checkbox, for now, we are going to do 3 color different. It will probably be put back in a different location. You, I, Xiuqin will have to talk about it.
  • The Size field needs to change it values when you change the units. See the GWT version.
  • We will have to break the size (text input, units, text feedback) our into it own component. Other parts of the application will need it. I forgot to tell you earlier. I don't know it that must be done right away, but in the next 2 or 3 weeks.
  • The NED or Simbad option is really part of the target panel. It needs to be put in there. I was not planning on you doing that. Just leave it there for now but don't do anything with it.
  • If there is an error, show a message to you user and don't hide the dialog. You can use PopupUtil.showInfoPopup

</div>
);
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We are trying to get away from tables for Layout. Using table to layout a table is fine but we are trying to use divs for other layout. Since I am out today, Loi could probably give you an overview of the best techniques we have been using.

  • display:'inline-block'
  • display: 'flex'

It will be a little more work at first but cleaner in the end.

@robyww
Copy link
Copy Markdown
Contributor

robyww commented Mar 15, 2016

ImageSelectPanel is a big file and will probably get bigger before you are done with all three tickets. You might want to consider breaking it up. You have to break out the size field anyway. However, there might be other parts as well.

@robyww
Copy link
Copy Markdown
Contributor

robyww commented Mar 15, 2016

Overall it is a good start. The code is clear and show a good understanding of the task. Good Job!

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.

2 participants