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 config checkbox to show/hide redirect URLs on Homepage #307

Open
wants to merge 5 commits into
base: unstable
Choose a base branch
from

Conversation

orelgs
Copy link

@orelgs orelgs commented Aug 16, 2024

This gives users more personalization for what is shown on the home page :)

Copy link

cla-bot bot commented Aug 16, 2024

We require contributors to sign our Contributor License Agreement. In order for us to review and merge your code, add yourself to the .clabot file as contributor, as a way of signing the CLA.

@azukaar
Copy link
Owner

azukaar commented Aug 19, 2024

Thanks for the contribution!
Upon reviewing there's an issue with the general idea: basically, I don't think a generic "show redirect" is enough for this. Instead, I think the check should be per-URL.
There is already a "Hide from dashboard" option in URL, but the URLs that are redirect are ignored.
I think the PR should make it so redirects URLs are not ignored, but "hide from dashboard" is true by default when creating a redirect
I am just not exactly sure how to implement this because when you reach the form to create a URL, the ttoe gas bit been selected yet

Copy link

cla-bot bot commented Aug 20, 2024

We require contributors to sign our Contributor License Agreement. In order for us to review and merge your code, add yourself to the .clabot file as contributor, as a way of signing the CLA.

@orelgs
Copy link
Author

orelgs commented Aug 20, 2024

Thanks for the contribution! Upon reviewing there's an issue with the general idea: basically, I don't think a generic "show redirect" is enough for this. Instead, I think the check should be per-URL. There is already a "Hide from dashboard" option in URL, but the URLs that are redirect are ignored. I think the PR should make it so redirects URLs are not ignored, but "hide from dashboard" is true by default when creating a redirect I am just not exactly sure how to implement this because when you reach the form to create a URL, the ttoe gas bit been selected yet

I get what you're saying, then by this logic, changing the URL mode automatically decides the base value for HideFromDashboard, so I've implemented a simple onChange method for when a user changes the URL mode.
Now HideFromDashboard is automatically selected / deselected depending on the mode (selected if mode is redirect), the user can change this afterwards.
Also removed the auto-skip URL on homepage.

What do you say?

@azukaar
Copy link
Owner

azukaar commented Aug 21, 2024

Yes that looks better thanks! Could you remove the condition with the skip altogether? I dont think it will be required anymore since the value is always false
then it should be ready to merge

Copy link

cla-bot bot commented Aug 21, 2024

We require contributors to sign our Contributor License Agreement. In order for us to review and merge your code, add yourself to the .clabot file as contributor, as a way of signing the CLA.

@orelgs
Copy link
Author

orelgs commented Aug 21, 2024

Yes that looks better thanks! Could you remove the condition with the skip altogether? I dont think it will be required anymore since the value is always false then it should be ready to merge

Done, is this what you meant?

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