Why isn't this working?

Try reversing the if statements and the function. Like this:

script.Parent.MouseButton1Down:connect(function()
    if script.Parent.Parent.Parent.Open.Visible == false then
        script.Parent.Parent.Parent.Open.Visible = true
    else
        script.Parent.Parent.Parent.Open.Visible = false
    end
end)

Try that. If it doesn't work, let me know and i'll see what I can do. Hope I helped :P

The way you wrote this code makes it so that you have a single handler for the MouseButton1Down event, and that handler only does a single thing.

This if statement will run as soon as the game starts and never run again.

If the gui is invisible when this if statement runs, you make it visible each time you click it.

If the gui is visible when this if statement runs, you make it invisible each time you click it.

There is absolutely nothing here to in any way tell the computer to make the gui switch between being visible and invisible.

To accomplish what you want, you must create a variable that will allow you to keep track whether the gui is open or closed. You will also need to only use one event handler.

local visible = false
local gui = script.Parent.Parent.Parent.Open

script.Parent.MouseButton1Down:connect(function()
    if not visible then --equivalent to: visible == false
        visible = true
        gui.Visible = true
    else
        visible = false
        gui.Visible = false
    end
end)