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

Fix redirect to homepage in read page #89

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

huyng12
Copy link

@huyng12 huyng12 commented Aug 17, 2021

Fix #83

@lqt93 lqt93 requested a review from huytd August 17, 2021 05:57
@huy-qn
Copy link
Contributor

huy-qn commented Aug 18, 2021

Cái fix này bỏ luôn chức năng của nút back.

Mình nghĩ để cái lịnk back to home ở cái logo (nhấn vô logo thì mới về trang chủ), còn thay typo cái nút "quay về trang chủ" trong bài gửi bằng 1 cái "icon" back thôi. Hoặc là bỏ luôn đi, dùng navigation của browsers/device cho tiện.

@huyng12
Copy link
Author

huyng12 commented Aug 18, 2021

Cái fix này bỏ luôn chức năng của nút back.

Mình nghĩ để cái lịnk back to home ở cái logo (nhấn vô logo thì mới về trang chủ), còn thay typo cái nút "quay về trang chủ" trong bài gửi bằng 1 cái "icon" back thôi. Hoặc là bỏ luôn đi, dùng navigation của browsers/device cho tiện.

Mình nghĩ là bạn đang so sánh changes của mình với code gốc, nếu như xét về ngữ nghĩa của "Quay về trang chủ" thì fix của mình bỏ đi những dòng code "thừa" (và không đúng).

@huytd
Copy link
Collaborator

huytd commented Aug 18, 2021

Please check this comment about the different use cases of back and push('/') https://github.com/webuild-community/federated-blog/pull/27/files#r659125981

@huy-qn
Copy link
Contributor

huy-qn commented Aug 18, 2021

Please check this comment about the different use cases of back and push('/') https://github.com/webuild-community/federated-blog/pull/27/files#r659125981

This doesn't work quite well with the "read a random article" feature. For users who read random articles continuously, these articles get pushed into the history queue, so the "back to home" button doesn't work as it say in this case, hence the issue happened here.

@huytd
Copy link
Collaborator

huytd commented Aug 27, 2021

We can add some query param to indicate that the user is in random read mode, I think

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.

Cannot back to Home page when Randomly read an article several times
3 participants