What is wrong with this text button script?

This script is supposed to make a GUI visible is it is not, or invisible if it is. It is a normal script inside a frame, inside a TextButton, inside a ScreenG UI inside the Starter GUI.

local button = script.Parent.Parent
local frame = script.Parent
function onMouseButton1Down()
	--Hide the entire frame, don't remove it!
			if frame.Visible == false then
			frame.Visible = true
			if frame.Visible == true then
			frame.Visible = false
		end
	end
end
button.MouseButton1Down:connect(onMouseButton1Down)

Would it be better to make it elseif instead of he second if? Please include code in your answer. Thanks.

That would be better :

local button = script.Parent.Parent
local frame = script.Parent
function onMouseButton1Down()
            if frame.Visible == false then
            frame.Visible = true
            elseif frame.Visible == true then
            frame.Visible = false
        end
end
button.MouseButton1Down:connect(onMouseButton1Down)

But don't do that:

            if frame.Visible == false then
            frame.Visible = true
	    end
            if frame.Visible == true then
            frame.Visible = false
	    end

Else your frame will turn visible and instantly turn invisible back.