Skip to content

Conversation

@shunter
Copy link
Contributor

@shunter shunter commented Feb 1, 2013

  • General cleanup, reformatting, refactoring, code standards, etc.
  • Fixed package configuration for both Dojo and RequireJS
    • By using the right baseUrl, we don't need to list all the packages
    • Dojo config is supposed to be defined as var dojoConfig, RequireJS config is supposed to be done with require.config
  • Centralize CSS that all examples use in a shared bucket.css
  • Fix the loading display
    • Separate it from the toolbar
    • Implement it as a more visible overlay which is shown/hidden via CSS
    • All examples have a sandcastle-loading class on their body which can be used to hide other UI while loading
    • Instead of having to set the toolbar's innerHTML to '' randomly before putting buttons there, examples now call Sandcastle.finishedLoading(); at the end of the function, which more clearly describes the intent
  • Disable the keyboard input for the Dojo TabContainer that holds the JS/HTML editors so that ctrl-home and ctrl-end work like they're supposed to
  • When switching tabs between JS/HTML, focus the editors automatically
  • Copy all attributes from the example's body onto the bucket's body, instead of just class/title, and use regexes instead of fragile indexOfs
  • Clean up and simplify examples themselves, and only use the Dojo modules we need.
  • Fix Skybox in a number of examples.

* General cleanup, reformatting, refactoring, code standards, etc.
* Fixed package configuration for both Dojo and RequireJS
   * By using the right baseUrl, we don't need to list all the packages
   * Dojo config is supposed to be defined as var dojoConfig, RequireJS config is supposed to be done with require.config
* Centralize CSS that all examples use in a shared bucket.css
* Fix the loading display
   * Separate it from the toolbar
   * Implement it as a more visible overlay which is shown/hidden via CSS
   * All examples have a sandcastle-loading class on their body which can be used to hide other UI while loading
   * Instead of having to set the toolbar's innerHTML to '' randomly before putting buttons there, examples now call Sandcastle.finishedLoading(); at the end of the function, which more clearly describes the intent
* Disable the keyboard input for the Dojo TabContainer that holds the JS/HTML editors so that ctrl-home and ctrl-end work like they're supposed to
* When switching tabs between JS/HTML, focus the editors automatically
* Copy all attributes from the example's body onto the bucket's body, instead of just class/title, and use regexes instead of fragile indexOfs
* Clean up and simplify examples themselves, and only use the Dojo modules we need.
* Fix Skybox in a number of examples.
@shunter
Copy link
Contributor Author

shunter commented Feb 1, 2013

You probably want to do ?w=1 on the diff to reduce the whitespace noise

@pjcozzi
Copy link
Contributor

pjcozzi commented Feb 1, 2013

I'm not the best person to review this; let's wait until after b13.

@mramato
Copy link
Contributor

mramato commented Feb 1, 2013

@emackey is probably the only one qualified. After we merge this in, we should update the website since it has a lot of good changes.

@pjcozzi
Copy link
Contributor

pjcozzi commented Feb 1, 2013

Yeap, I'll update it.

@pjcozzi
Copy link
Contributor

pjcozzi commented Feb 1, 2013

The Cesium demo on the website needs to be updated to reflect the Dojo changes. Anyone game to do?

@shunter
Copy link
Contributor Author

shunter commented Feb 1, 2013

I'll take a look. There's some weirdness going on with that demo.

@pjcozzi
Copy link
Contributor

pjcozzi commented Feb 1, 2013

Thanks @shunter

Copy link
Contributor

Choose a reason for hiding this comment

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

Pardon my ignorance of Dojo keyboard handling. What does this do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

StackContainer (a parent class of TabContainer) has an _onKeyPress method that receives DOM keypress events and publishes them to a controller that listens for hotkeys and switches children around. Disappointingly, there's no programmatic API to disable keyboard shortcuts on a StackContainer, so this replaces the _onKeyPress method on the instance of TabContainer created declaratively from the HTML with a function that does nothing, disconnecting the controller from the DOM keypress events.

Yeah, I know this sucks.

@emackey
Copy link
Contributor

emackey commented Feb 1, 2013

Looks like the jQuery Sandcastle demo has the old loading banner, should it be upgraded to the new one? Or was there a reason it was left alone?

@shunter
Copy link
Contributor Author

shunter commented Feb 1, 2013

Yeah, I'll add that to the jQuery one. It was mostly an oversight.

emackey and others added 4 commits February 1, 2013 16:08
…ading` class.

Even if a particular demo doesn't need this class, it should still
be included for consistency with the bucket, so that when a user
modifies and re-exports it, the only diffs will be user mods.
@emackey
Copy link
Contributor

emackey commented Feb 1, 2013

Should Minimalist and Hello World use the new loading banner?

@shunter
Copy link
Contributor Author

shunter commented Feb 1, 2013

I had decided not to, since they load quickly. Minimalist actually loads inline, so I don't think the loading banner would even be drawn. Hello World maybe should, since it's pulling modules with RequireJS.

@emackey
Copy link
Contributor

emackey commented Feb 1, 2013

OK. Let's add it to Hello World (for first impressions) and leave it out of Minimalist.

@emackey
Copy link
Contributor

emackey commented Feb 1, 2013

Looks good! Merging.

emackey added a commit that referenced this pull request Feb 1, 2013
@emackey emackey merged commit d8f2ff2 into master Feb 1, 2013
@emackey emackey deleted the sandcastle_cleanup branch February 1, 2013 22:00
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.

5 participants