Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add logout feature #1301

Merged
merged 4 commits into from
Oct 31, 2024
Merged

add logout feature #1301

merged 4 commits into from
Oct 31, 2024

Conversation

lpcheng1208
Copy link
Contributor

add logout feature

@lpcheng1208
Copy link
Contributor Author

image image

main.go Outdated
@@ -183,6 +183,7 @@ func runWebServer() error {
http.HandleFunc("/static/", web.AuthAssert(staticFsFunc))
http.HandleFunc("/favicon.ico", web.AuthAssert(faviconFsFunc))
http.HandleFunc("/login", web.AuthAssert(web.Login))
http.HandleFunc("/logout", web.AuthAssert(web.Logout))
Copy link
Owner

Choose a reason for hiding this comment

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

web.Auth(),要验证登录了才能访问退出登录

web/writing.html Outdated
@@ -42,6 +42,10 @@
id="themeButton"
></span>
<span class="badge badge-secondary">{{.Version}}</span>
<a href="/logout" class="btn btn-danger btn-sm btn-logout">
Copy link
Owner

Choose a reason for hiding this comment

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

./logout 有些可能用了二级目录

web/logout.go Outdated
http.SetCookie(w, &expiredCookie)

// 重定向用户到登录页面
http.Redirect(w, r, "/login", http.StatusFound)
Copy link
Owner

Choose a reason for hiding this comment

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

./login同其它地方


.btn-logout i {
margin-right: 5px; /* 图标和文字之间的间距 */
Copy link
Owner

Choose a reason for hiding this comment

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

加了5个px,看上去文字没居中。用文字还是退出登录按钮,放一个红色在此处感觉有些有显眼了

Copy link
Contributor Author

Choose a reason for hiding this comment

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

加了5个px,看上去文字没居中。用文字还是退出登录按钮,放一个红色在此处感觉有些有显眼了

对前端不是很熟悉,去除 5px ,用文字还是退出登录按钮 后面这个是啥意思?红色确实有点显眼 有什么好建议吗

Copy link
Owner

Choose a reason for hiding this comment

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

就文字吧,简单点

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image 这样呢?

Copy link
Owner

Choose a reason for hiding this comment

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

可以,比之前好看些

Copy link
Contributor Author

Choose a reason for hiding this comment

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

纯白边框看上去有点显眼

这个是借鉴了 Github 未登录状态的样式,感觉还行

Copy link
Owner

Choose a reason for hiding this comment

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

纯白边框看上去有点显眼

这个是借鉴了 Github 未登录状态的样式,感觉还行

好,也可以吧

Copy link
Owner

Choose a reason for hiding this comment

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

要不一起把日志那个改了,保持一致?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

要不一起把日志那个改了,保持一致?

我试试

Copy link
Owner

Choose a reason for hiding this comment

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

试了下,还是保持之前那个日志按钮

web/writing.html Outdated
Comment on lines 45 to 47
<a href="./logout" class="action-button logout-button" data-i18n="Logout">
注销
</a>
Copy link
Owner

Choose a reason for hiding this comment

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

向前缩进,注销文字保持和其它类似,为英文

Copy link
Contributor Author

Choose a reason for hiding this comment

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

向前缩进,注销文字保持和其它类似,为英文

哈,这个倒是改了,但是Logs 也推上去了,确实感觉还是之前的比较 OK

Copy link
Owner

Choose a reason for hiding this comment

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

也可以

Copy link
Contributor Author

Choose a reason for hiding this comment

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

也可以

Logs 需要搞回去吗? 还是你push 一个上来?我刚想改回去,发现已经merge了

Copy link
Owner

Choose a reason for hiding this comment

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

改不改都可以,你想改就在来个

@jeessy2 jeessy2 merged commit abef25c into jeessy2:master Oct 31, 2024
jeessy2 added a commit that referenced this pull request Oct 31, 2024
@jeessy2
Copy link
Owner

jeessy2 commented Oct 31, 2024

失误了,应该要覆盖cookieInSystem

@jeessy2
Copy link
Owner

jeessy2 commented Oct 31, 2024

现在才发现,失误

	// 覆盖cookieInSystem
	cookieInSystem = &http.Cookie{
		Name:     cookieName,
		Value:    "",
		Path:     "/",
		Expires:  time.Unix(0, 0), // 设置为过期时间
		MaxAge:   -1,              // 立即删除该 Cookie
		HttpOnly: true,
	}
	// 设置过期的 Cookie
	http.SetCookie(w, cookieInSystem)

@lpcheng1208
Copy link
Contributor Author

jeessy2 added a commit that referenced this pull request Oct 31, 2024

啥意思?验证了一下应该没啥问题啊

@jeessy2
Copy link
Owner

jeessy2 commented Oct 31, 2024

只清理了页面的,系统存的cookie没清理

@lpcheng1208
Copy link
Contributor Author

只清理了页面的,系统存的cookie没清理

完啦,我没看到系统的哪里存了

@jeessy2
Copy link
Owner

jeessy2 commented Oct 31, 2024

现在才发现,失误


	// 覆盖cookieInSystem

	cookieInSystem = &http.Cookie{

		Name:     cookieName,

		Value:    "",

		Path:     "/",

		Expires:  time.Unix(0, 0), // 设置为过期时间

		MaxAge:   -1,              // 立即删除该 Cookie

		HttpOnly: true,

	}

	// 设置过期的 Cookie

	http.SetCookie(w, cookieInSystem)

这样就可以了

@lpcheng1208
Copy link
Contributor Author

现在才发现,失误


	// 覆盖cookieInSystem

	cookieInSystem = &http.Cookie{

		Name:     cookieName,

		Value:    "",

		Path:     "/",

		Expires:  time.Unix(0, 0), // 设置为过期时间

		MaxAge:   -1,              // 立即删除该 Cookie

		HttpOnly: true,

	}

	// 设置过期的 Cookie

	http.SetCookie(w, cookieInSystem)

这样就可以了

image 你说的是这个吗?

@lpcheng1208
Copy link
Contributor Author

cookieInSystem

前面确实没注意 存了个全局 😂

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.

2 participants