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

Monospace font in text inputs #149

Merged
merged 1 commit into from
Dec 5, 2018
Merged

Monospace font in text inputs #149

merged 1 commit into from
Dec 5, 2018

Conversation

Roy-Orbison
Copy link
Contributor

To make code more readable on the right panel.

@eliihen
Copy link
Member

eliihen commented Nov 28, 2018

👋 Hi, thanks a lot for the PR!

I don't have time to look at this right now, but I'll get back to you - probably this weekend

Copy link
Member

@eliihen eliihen left a comment

Choose a reason for hiding this comment

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

I just loaded the code and had a look at it - looks great!

The only thing I found that was missing was monospaced fonts in the right column's <textarea> (the custom request body field). If you add this I will happily merge this!

@Roy-Orbison
Copy link
Contributor Author

Done. Only missed the main reason I thought of making the change 🙃.

Hope changing the URL input's type attribute is okay. Didn't look like it was critical.

@eliihen
Copy link
Member

eliihen commented Dec 4, 2018

URL validation

Hope changing the URL input's type attribute is okay. Didn't look like it was critical.

I would ordinarily say yeah, that's a great idea as it is more semantically correct. However it results in the browser validation engine kicking URLs without a protocol, for example like the above.

RESTED has a built in feature for prepending http:// to URLs without a protocol, so this is unfortunately a regression. If you want to look into this, you're free to have a look at it (for example by setting novalidate on the form), but it's outside the scope of this PR. So for now just take it out, and we can merge this 🙂

...on the right panel only.

Signed-off-by: Roy-Orbison <Roy-Orbison@users.noreply.github.com>
@Roy-Orbison
Copy link
Contributor Author

Fair enough. I'd be changing the value of the input to show the prepended scheme as a gentle reminder that they're not using https, then URL validation wouldn't be an issue.

@eliihen eliihen merged commit 322aa4b into RESTEDClient:master Dec 5, 2018
@eliihen
Copy link
Member

eliihen commented Dec 5, 2018

Just merged, thanks a lot! ❤️

I'm going to wait some days before releasing a new version to allow potential other changes to ship with this. Stay tuned

@eliihen eliihen added this to the 2.3.0 milestone Dec 5, 2018
@eliihen eliihen mentioned this pull request Dec 9, 2018
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