Added some notes and an assertion to Component::grabKeyboardFocus(), to help people avoid a common mistake, which is trying to grab the focus of not-yet-visible components.

extras/Projucer/Source/Project/jucer_ProjectContentComponent.cpp

@@ -728,7 +728,7 @@ bool ProjectContentComponent::showDocument (OpenDocumentManager::Document* doc,
 
     bool opened = setEditorComponent (doc->createEditor(), doc);
 
-    if (opened && grabFocus)
+    if (opened && grabFocus && isShowing())
         contentView->grabKeyboardFocus();
 
     return opened;

modules/juce_gui_basics/components/juce_Component.cpp

@@ -69,7 +69,7 @@ public:
         if (checker.shouldBailOut())
             return;
 
-        if (MouseListenerList* const list = comp.mouseListeners)
+        if (auto* list = comp.mouseListeners.get())
         {
             for (int i = list->listeners.size(); --i >= 0;)
             {
@@ -84,7 +84,7 @@ public:
 
         for (Component* p = comp.parentComponent; p != nullptr; p = p->parentComponent)
         {
-            MouseListenerList* const list = p->mouseListeners;
+            auto* list = p->mouseListeners.get();
 
             if (list != nullptr && list->numDeepMouseListeners > 0)
             {
@@ -106,7 +106,7 @@ public:
     static void sendWheelEvent (Component& comp, Component::BailOutChecker& checker,
                                 const MouseEvent& e, const MouseWheelDetails& wheel)
     {
-        if (MouseListenerList* const list = comp.mouseListeners)
+        if (auto* list = comp.mouseListeners.get())
         {
             for (int i = list->listeners.size(); --i >= 0;)
             {
@@ -144,9 +144,8 @@ private:
     Array<MouseListener*> listeners;
     int numDeepMouseListeners;
 
-    class BailOutChecker2
+    struct BailOutChecker2
     {
-    public:
         BailOutChecker2 (Component::BailOutChecker& boc, Component* const comp)
             : checker (boc), safePointer (comp)
         {
@@ -361,7 +360,7 @@ struct Component::ComponentHelpers
     template <typename PointOrRect>
     static PointOrRect convertFromDistantParentSpace (const Component* parent, const Component& target, const PointOrRect& coordInParent)
     {
-        const Component* const directParent = target.getParentComponent();
+        auto* directParent = target.getParentComponent();
         jassert (directParent != nullptr);
 
         if (directParent == parent)
@@ -389,7 +388,7 @@ struct Component::ComponentHelpers
         if (target == nullptr)
             return p;
 
-        const Component* const topLevelComp = target->getTopLevelComponent();
+        auto* topLevelComp = target->getTopLevelComponent();
 
         p = convertFromParentSpace (*topLevelComp, p);
 
@@ -504,7 +503,7 @@ void Component::setName (const String& name)
         componentName = name;
 
         if (flags.hasHeavyweightPeerFlag)
-            if (ComponentPeer* const peer = getPeer())
+            if (auto* peer = getPeer())
                 peer->setTitle (name);
 
         BailOutChecker checker (this);
@@ -554,7 +553,7 @@ void Component::setVisible (bool shouldBeVisible)
 
             if (safePointer != nullptr && flags.hasHeavyweightPeerFlag)
             {
-                if (ComponentPeer* const peer = getPeer())
+                if (auto* peer = getPeer())
                 {
                     peer->setVisible (shouldBeVisible);
                     internalHierarchyChanged();
@@ -583,7 +582,7 @@ bool Component::isShowing() const
     if (parentComponent != nullptr)
         return parentComponent->isShowing();
 
-    if (const ComponentPeer* const peer = getPeer())
+    if (auto* peer = getPeer())
         return ! peer->isMinimised();
 
     return false;
@@ -592,7 +591,7 @@ bool Component::isShowing() const
 //==============================================================================
 void* Component::getWindowHandle() const
 {
-    if (const ComponentPeer* const peer = getPeer())
+    if (auto* peer = getPeer())
         return peer->getNativeHandle();
 
     return nullptr;
@@ -865,7 +864,7 @@ void Component::reorderChildInternal (const int sourceIndex, const int destIndex
 {
     if (sourceIndex != destIndex)
     {
-        Component* const c = childComponentList.getUnchecked (sourceIndex);
+        auto* c = childComponentList.getUnchecked (sourceIndex);
         jassert (c != nullptr);
         c->repaintParent();
 
@@ -884,7 +883,7 @@ void Component::toFront (const bool setAsForeground)
 
     if (flags.hasHeavyweightPeerFlag)
     {
-        if (ComponentPeer* const peer = getPeer())
+        if (auto* peer = getPeer())
         {
             peer->toFront (setAsForeground);
 
@@ -919,7 +918,9 @@ void Component::toFront (const bool setAsForeground)
         if (setAsForeground)
         {
             internalBroughtToFront();
-            grabKeyboardFocus();
+
+            if (isShowing())
+                grabKeyboardFocus();
         }
     }
 }
@@ -955,8 +956,8 @@ void Component::toBehind (Component* const other)
 
             if (other->isOnDesktop())
             {
-                ComponentPeer* const us = getPeer();
-                ComponentPeer* const them = other->getPeer();
+                auto* us = getPeer();
+                auto* them = other->getPeer();
 
                 jassert (us != nullptr && them != nullptr);
                 if (us != nullptr && them != nullptr)
@@ -1004,7 +1005,7 @@ void Component::setAlwaysOnTop (const bool shouldStayOnTop)
 
         if (isOnDesktop())
         {
-            if (ComponentPeer* const peer = getPeer())
+            if (auto* peer = getPeer())
             {
                 if (! peer->setAlwaysOnTop (shouldStayOnTop))
                 {
@@ -1153,7 +1154,7 @@ void Component::setBounds (const int x, const int y, int w, int h)
         flags.isResizeCallbackPending = wasResized;
 
         if (flags.hasHeavyweightPeerFlag)
-            if (ComponentPeer* const peer = getPeer())
+            if (auto* peer = getPeer())
                 peer->updateBounds();
 
         sendMovedResizedMessagesIfPending();
@@ -1413,7 +1414,7 @@ bool Component::contains (Point<int> point)
             return parentComponent->contains (ComponentHelpers::convertToParentSpace (*this, point));
 
         if (flags.hasHeavyweightPeerFlag)
-            if (const ComponentPeer* const peer = getPeer())
+            if (auto* peer = getPeer())
                 return peer->contains (ComponentHelpers::localPositionToRawPeerPos (*this, point), true);
     }
 
@@ -1425,8 +1426,8 @@ bool Component::reallyContains (Point<int> point, const bool returnTrueIfWithinA
     if (! contains (point))
         return false;
 
-    Component* const top = getTopLevelComponent();
-    const Component* const compAtPosition = top->getComponentAt (top->getLocalPoint (this, point));
+    auto* top = getTopLevelComponent();
+    auto* compAtPosition = top->getComponentAt (top->getLocalPoint (this, point));
 
     return (compAtPosition == this) || (returnTrueIfWithinAChild && isParentOf (compAtPosition));
 }
@@ -1538,7 +1539,7 @@ Component* Component::removeChildComponent (const int index, bool sendParentEven
     // thread, you'll need to use a MessageManagerLock object to make sure it's thread-safe.
     ASSERT_MESSAGE_MANAGER_IS_LOCKED_OR_OFFSCREEN
 
-    Component* const child = childComponentList [index];
+    auto* child = childComponentList [index];
 
     if (child != nullptr)
     {
@@ -1619,7 +1620,7 @@ Component* Component::findChildWithID (StringRef targetID) const noexcept
 {
     for (int i = childComponentList.size(); --i >= 0;)
     {
-        Component* const c = childComponentList.getUnchecked(i);
+        auto* c = childComponentList.getUnchecked(i);
         if (c->componentID == targetID)
             return c;
     }
@@ -1799,7 +1800,7 @@ bool Component::isCurrentlyModal (bool onlyConsiderForemostModalComponent) const
 
 bool Component::isCurrentlyBlockedByAnotherModalComponent() const
 {
-    Component* const mc = getCurrentlyModalComponent();
+    auto* mc = getCurrentlyModalComponent();
 
     return ! (mc == nullptr || mc == this || mc->isParentOf (this)
                || mc->canModalEventBeSentToComponent (this));
@@ -1875,7 +1876,7 @@ void Component::alphaChanged()
 {
     if (flags.hasHeavyweightPeerFlag)
     {
-        if (ComponentPeer* const peer = getPeer())
+        if (auto* peer = getPeer())
             peer->setAlpha (getAlpha());
     }
     else
@@ -1929,7 +1930,7 @@ void Component::internalRepaintUnchecked (Rectangle<int> area, const bool isEnti
             // thread, you'll need to use a MessageManagerLock object to make sure it's thread-safe.
             ASSERT_MESSAGE_MANAGER_IS_LOCKED
 
-            if (ComponentPeer* const peer = getPeer())
+            if (auto* peer = getPeer())
             {
                 // Tweak the scaling so that the component's integer size exactly aligns with the peer's scaled size
                 const Rectangle<int> peerBounds (peer->getBounds());
@@ -2327,7 +2328,7 @@ bool Component::canModalEventBeSentToComponent (const Component*)
 
 void Component::internalModalInputAttempt()
 {
-    if (Component* const current = getCurrentlyModalComponent())
+    if (auto* current = getCurrentlyModalComponent())
         current->inputAttemptWhenModal();
 }
 
@@ -2341,7 +2342,7 @@ void Component::postCommandMessage (const int commandId)
 
         void messageCallback() override
         {
-            if (Component* c = target.get())
+            if (auto* c = target.get())
                 c->handleCommandMessage (commandId);
         }
 
@@ -2684,7 +2685,7 @@ void Component::internalBroughtToFront()
 
     // When brought to the front and there's a modal component blocking this one,
     // we need to bring the modal one to the front instead..
-    if (Component* const cm = getCurrentlyModalComponent())
+    if (auto* cm = getCurrentlyModalComponent())
         if (cm->getTopLevelComponent() != getTopLevelComponent())
             ModalComponentManager::getInstance()->bringModalComponentsToFront (false); // very important that this is false, otherwise in Windows,
                                                                                        // non-front components can't get focus when another modal comp is
@@ -2793,7 +2794,7 @@ void Component::takeKeyboardFocus (const FocusChangeType cause)
     if (currentlyFocusedComponent != this)
     {
         // get the focus onto our desktop window
-        if (ComponentPeer* const peer = getPeer())
+        if (auto* peer = getPeer())
         {
             const WeakReference<Component> safePointer (this);
             peer->grabFocus();
@@ -2839,7 +2840,7 @@ void Component::grabFocusInternal (const FocusChangeType cause, const bool canTr
 
                 if (traverser != nullptr)
                 {
-                    Component* const defaultComp = traverser->getDefaultComponent (this);
+                    auto* defaultComp = traverser->getDefaultComponent (this);
                     traverser = nullptr;
 
                     if (defaultComp != nullptr)
@@ -2867,6 +2868,12 @@ void Component::grabKeyboardFocus()
     ASSERT_MESSAGE_MANAGER_IS_LOCKED
 
     grabFocusInternal (focusChangedDirectly, true);
+
+    // A component can only be focused when it's actually on the screen!
+    // If this fails then you're probably trying to grab the focus before you've
+    // added the component to a parent or made it visible. Or maybe one of its parent
+    // components isn't yet visible.
+    jassert (isShowing() || isOnDesktop());
 }
 
 void Component::moveKeyboardFocusToSibling (const bool moveToNext)
@@ -2881,8 +2888,8 @@ void Component::moveKeyboardFocusToSibling (const bool moveToNext)
 
         if (traverser != nullptr)
         {
-            Component* const nextComp = moveToNext ? traverser->getNextComponent (this)
-                                                   : traverser->getPreviousComponent (this);
+            auto* nextComp = moveToNext ? traverser->getNextComponent (this)
+                                        : traverser->getPreviousComponent (this);
             traverser = nullptr;
 
             if (nextComp != nullptr)
@@ -2918,13 +2925,13 @@ Component* JUCE_CALLTYPE Component::getCurrentlyFocusedComponent() noexcept
 
 void JUCE_CALLTYPE Component::unfocusAllComponents()
 {
-    if (Component* c = getCurrentlyFocusedComponent())
+    if (auto* c = getCurrentlyFocusedComponent())
         c->giveAwayFocus (true);
 }
 
 void Component::giveAwayFocus (const bool sendFocusLossEvent)
 {
-    Component* const componentLosingFocus = currentlyFocusedComponent;
+    auto* componentLosingFocus = currentlyFocusedComponent;
     currentlyFocusedComponent = nullptr;
 
     if (sendFocusLossEvent && componentLosingFocus != nullptr)
@@ -2966,7 +2973,7 @@ void Component::sendEnablementChangeMessage()
 
     for (int i = getNumChildComponents(); --i >= 0;)
     {
-        if (Component* const c = getChildComponent (i))
+        if (auto* c = getChildComponent (i))
         {
             c->sendEnablementChangeMessage();
 

modules/juce_gui_basics/components/juce_Component.h

@@ -1208,6 +1208,11 @@ public:
           parent instead, unless it's a top-level component without a parent,
           in which case it just takes the focus itself.
 
+        Important note! It's obviously not possible for a component to be focused
+        unless it's actually visible, on-screen, and inside a window that is also
+        visible. So there's no point trying to call this in the component's own
+        constructor or before all of its parent hierarchy has been fully instantiated.
+
         @see setWantsKeyboardFocus, getWantsKeyboardFocus, hasKeyboardFocus,
              getCurrentlyFocusedComponent, focusGained, focusLost,
              keyPressed, keyStateChanged