In #1064600: Maximizing editors places toolbar under Drupal's toolbar and #1898210: Administration menu overlaps editors in fullscreen mode we had to introduce a bit of code to hide Drupal toolbars, like the ones provided by the Toolbar and Aministration menu modules, while the Overlay module is enabled.

In the last issue sun suggested we turn those snippets into separate functions to decrease code duplication.


TwoD’s picture

Status: Active » Needs review
3.91 KB

And here's an initial patch for that.

(Note: The code is the same for all of D5, D6 and D7 even though the Overlay only exists in D7, just in case someone backports Overlay and Toolbar, and to keep maintenance overhead down. Admin menu does exist before Drupal 7.)

sun’s picture

Thanks! I think this looks good.

The only nitpick I have is that "preFullscreen" and "postFullscreen" both sound a bit odd as method names. I first thought about "prepareFullscreen", but then couldn't find a sensible opposite name. The second alternative was setupFullscreen and teardownFullscreen. Ultimately, I concluded whether we could make them sound like event handlers; e.g., onBeforeFullscreen, onAfterFullscreen? Or similar?

danbohea’s picture

openFullscreen / closeFullscreen ?

Pre/Post seems fine to me.

TwoD’s picture

I had the same feeling about the names... A catch is that we can't really guarantee the actions are always performed before going to fullscreen and after exiting fullscreen, as that depends a bit on when the implementation can react on the "events". I ended up reasoning that if we despite that fact still think about it as what needs to be done before/after fullscreen mode is in effect, what to do in the functions is fairly obvious. It's also going to happen so fast nobody's likely to notice the minimal delay...

I could go with any of the names mentioned so far. But if to nitpick, maybe we should pick names that don't imply exactly when the callback will be called, like onFullscreenEnter and onFullscreenExit?

danbohea’s picture

Good explanation.

+1 for onFullscreenEnter and onFullscreenExit.

TwoD’s picture

Re-rolled with the names from #6 and changed "before going to fullscreen mode" to "when going to fullscreen mode".

mglaman’s picture

Issue summary: View changes
4.08 KB

Reroll against 2.x HEAD.

mglaman’s picture

Status: Needs review » Reviewed & tested by the community

Tested and full screen / maximizing works. Nice to use utility functions.