-
Notifications
You must be signed in to change notification settings - Fork 371
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 param labels #519
Add param labels #519
Conversation
Codecov Report
@@ Coverage Diff @@
## master #519 +/- ##
==========================================
+ Coverage 96.55% 96.58% +0.02%
==========================================
Files 52 52
Lines 2238 2255 +17
==========================================
+ Hits 2161 2178 +17
Misses 77 77
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Hello friends! Any idea when this will be merged or if it may need some changes to be ready? This is a small change and shouldn't break anything. I'm already using my branch on my project and works great and would really like to see this merged on main to avoid adding the github branch on my requirements. The only thing that may be needed is that if you are embedding queries in your views (see this issue #485 ). You'll need to change the input label and value similar to how they have been implemented in the internal templates. See the diffs on explorer/templates/explorer/params.html (https://github.com/groveco/django-sql-explorer/blob/3d78a805d428cf4a6e5f10a38b1048a9f9b7496a/explorer/templates/explorer/params.html#L6) But this change would be done in any case to take advantage of the labels. Also I'm not really sure how many people are using this feature since the #485 didn't get any traction :( Thank you! |
@spapas Sorry - I'd missed this PR. I'm not using the app like this, but can at least take a look at the code and see it makes sense. It's sometimes a bit frustrating that users aren't more involved on here, because the app does get used by quite a lot of people; |
@marksweb thank you! Well no matter how you use the app I think that adding labels to your parameters will improve the use experience. Especially if you want to have nice non-english labels, i.e here's a query I use this with:
i prefer seeing "Υπηρεσία" instead of "authority" :) |
Hey @marksweb any news about the status of this PR ? Thank you! |
@spapas I'm waiting for pypi to merge the django 4.2 classifier to get a release together. This will go into that. |
This implements #486.