kh90909 / OakTerm

Serial terminal for the Digistump Oak over the Particle Cloud
8 stars 2 forks source link

UI rework start #1

Closed emcniece closed 8 years ago

emcniece commented 8 years ago

Lots to do here, but this might be a start for cleaning up the interface for this project.

Big changes:

Todo: (much more obviously, but some notable items)

I like where you're headed with the promise chaining - one of the problems that really hung me up was thinking about what framework might be most suitable for handling this quantity of callbacks. There's opportunity for integrating Bootstrap's classes more closely with the errors.

Hopefully you get the point that this is far from presentable - it's just a humble start at a UI rework. What do you think?

kh90909 commented 8 years ago

Looks great, much cleaner code. I had kludged together @digistump's mock-up and OakSoftAP just to get started, but was about to start switching over to bootstrap to implement the modals, among other things, so this is timely.

I'll take a detailed look at the pull request tonight/tomorrow and merge it, but in the meantime, do you want to divide up some of the tasks so that we don't end up duplicating our efforts?

It looks like the respective items on our ToDo/"not yet implemented" lists don't overlap, so perhaps that's a good place to start? Beyond that, let's create an issue to track this. When we start working on a feature, let's comment there to let the other know. Feel free to grab things off my list if you finish yours, just comment this way so that I know.

The next thing I'm going to do is implement setting variables and calling functions, and the associated UI modals.

emcniece commented 8 years ago

Sounds good - issues created!

I created an issue for further basic UI improvement, just in case. Did you want to merge this PR, or should I keep committing to it until we're in a more demo-able state?

I've stuck to a mostly-UI focus for the issues so far, but I'm definitely happy to dip in to some of the API related issues too.

kh90909 commented 8 years ago

Sorry, I didn't get around to merging this yet, but I plan to. It doesn't make sense for me to base changes on the original code since so much has changed.

But, I won't get to merging it until tomorrow, so feel free to add to the pull request until then.

One thing I noticed is that the changes broke auto scrolling on the content div. The relevant js is still being called, so it may be an issue with the new CSS affecting the scrolltop calculation.

emcniece commented 8 years ago

Got it - issue added, will follow up!

emcniece commented 8 years ago

New commit added with a better UI polish - feedback appreciated! Clear to merge on my end.