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 label tip for Base URL settings fields #1391

Merged
merged 1 commit into from
Nov 25, 2024

Conversation

jgillman
Copy link
Contributor

@jgillman jgillman commented Nov 20, 2024

Adds label tips for Base URL fields! Simple fix for #1390

Screenshot
CleanShot 2024-11-20 at 11 29 09

@jgillman jgillman force-pushed the add-label-tip-for-base-url-fields branch from 180126f to acf6afa Compare November 20, 2024 17:27
@jgillman
Copy link
Contributor Author

lol 🤦 I force-pushed because the label text I added was the opposite of what it should have been

Copy link
Collaborator

@ydkmlt84 ydkmlt84 left a comment

Choose a reason for hiding this comment

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

It is still showing as must have, instead of must NOT have.

Also, I believe it would be better if it simply said No Leading Slash. Since it is implied its information for the Base URL setting, due to the position.

@jgillman jgillman force-pushed the add-label-tip-for-base-url-fields branch from acf6afa to 6d4c951 Compare November 21, 2024 20:42
@jgillman
Copy link
Contributor Author

@ydkmlt84 Made the requested changes!

Not sure what happened with my first force-push ¯\_(ツ)_/¯

@jorenn92
Copy link
Owner

Wouldn't it be better for the server-side logic to check for a leading slash and remove it if present?

@jgillman
Copy link
Contributor Author

@jorenn92 Definitely would be better for the server to do it! There was some discussion in the issue I created and adding a label seemed like a decent interim solution that I could readily handle myself.

Not sure how much more involved it would be to properly handle on the back end.

@jgillman jgillman force-pushed the add-label-tip-for-base-url-fields branch from 6d4c951 to e5491fa Compare November 23, 2024 17:51
@jorenn92
Copy link
Owner

jorenn92 commented Nov 25, 2024

@jorenn92 Definitely would be better for the server to do it! There was some discussion in the issue I created and adding a label seemed like a decent interim solution that I could readily handle myself.

Not sure how much more involved it would be to properly handle on the back end.

Oh, I see! I missed that issue. I didn’t realize this was just a temporary solution. In that case, it works for me.
@ydkmlt84, If you’re okay with the requested change, then I’m fine with it too.

@ydkmlt84 ydkmlt84 force-pushed the add-label-tip-for-base-url-fields branch from e5491fa to 126ac26 Compare November 25, 2024 15:22
@ydkmlt84 ydkmlt84 enabled auto-merge November 25, 2024 15:23
Copy link
Collaborator

@ydkmlt84 ydkmlt84 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@ydkmlt84 ydkmlt84 merged commit 20aa428 into jorenn92:main Nov 25, 2024
3 checks passed
@jgillman jgillman deleted the add-label-tip-for-base-url-fields branch November 27, 2024 00:41
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.

3 participants