Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

[ARCH] For Review: Updated bootstrap to 2.3.1 and less to 1.3.3 (needed for bootstrap) #3672

Closed

Conversation

WebsiteDeveloper
Copy link
Contributor

This is part of the refactorings to upgrade jQuery.
I added for review, because this isn't quite ready for a merge because some UI styles changed. Maybe an Experience Designer could look at it.

@peterflynn
Copy link
Member

Flagging for architecture review since this may be a fairly disruptive change, and we also need to think about how Topcoat fits in.

Some other potential issues:

  • Glenn mentioned that upgrading LESS seems to break the Brackets UI pretty badly in some cases.
  • It looks like this may be reintroducing dialog show/hide animations, which in our earlier version of Bootstrap had severe race condition bugs. We'll have to be very careful to make sure they're not reintroduced here.

@ghost ghost assigned gruehle May 10, 2013
@njx
Copy link
Contributor

njx commented May 10, 2013

To @gruehle for [ARCH] tag. Please figure out who to assign to if you decide we should open it.

@WebsiteDeveloper
Copy link
Contributor Author

@peterflynn i pushed up a few fixes which should return the UI to the normal state.
As to the show/hide animations I actually enabled them manually by adding the hide and fade class to the dialogs so this could be easily removed.

@TuckerWhitehouse
Copy link
Contributor

@WebsiteDeveloper it looks like this introduced a bug with the shadows in the sidebar (above index.html in both the working set and project panel) as well as the shadow above the project panel (div.scroller-shadow.top)
screenshot-pr3672

@WebsiteDeveloper
Copy link
Contributor Author

@TuckerWhitehouse thanks for the notice, will look at it.

@WebsiteDeveloper
Copy link
Contributor Author

@TuckerWhitehouse i pushed a fix which you might want to check out.

@TuckerWhitehouse
Copy link
Contributor

@WebsiteDeveloper Perfect! Thank you :)

@gruehle
Copy link
Member

gruehle commented May 10, 2013

@WebsiteDeveloper thanks for doing this work!

@peterflynn this branch does not have the visual problems I was seeing when I tried to update to less 1.3.3. I tried just updating less, which could have been part of the problem.

I am seeing a couple more problems:

  • JSLint results and "Find in Files" results don't have zebra striping, and look too condensed.
  • The "Install Extension" dialog has a horizontal scrollbar.
  • The icon next to "Read more" in Quick Docs is missing.
  • There are a few unit test failures. One with context menus, and several with the "Install Extension" dialog.

Everything else I checked looks fine. It would be good to check out some of the popular extensions to make sure they still basically work.

@WebsiteDeveloper
Copy link
Contributor Author

@gruehle pushed fixes for the problems you mentioned.

@gruehle
Copy link
Member

gruehle commented May 14, 2013

@WebsiteDeveloper looks much better - thanks!

We definitely want this change, but will not be able to pull it in this sprint. Hopefully we can get to it early in the next one, which starts on May 22nd. We'd like to get the jQuery 2.0 update done at the same time.

@WebsiteDeveloper
Copy link
Contributor Author

@gruehle i wonder if i should put up a version of this rebased on current master and then again look for regressions because as far as i know a few changes to the bootstrap style overrides have been made.

@gruehle
Copy link
Member

gruehle commented May 23, 2013

@WebsiteDeveloper - yes, that would be great. Thanks!

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

Successfully merging this pull request may close these issues.

None yet

5 participants