-
Notifications
You must be signed in to change notification settings - Fork 37
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 read_only_columns to TableConfig #289
Conversation
@sinisaos Thanks for this. It looks like we're hiding inputs if they're marked as readonly, but another option is to set the https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/readonly You should be able to do this using Vue:
|
@dantownsend I already tried that but that doesn't work. I think hidding field (to disable the editing of certain form fields) is not a bad solution because we have, in addition to input fields, various selects, DurationWidget, ChoiceWidget, FlatPickr, etc. where the If you want I can make another commit to make the fields visible but readonly or disabled, so we can see what works best? |
@dantownsend With the latest commit the field should be visible but |
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## master #289 +/- ##
==========================================
+ Coverage 93.31% 93.38% +0.07%
==========================================
Files 5 5
Lines 359 363 +4
==========================================
+ Hits 335 339 +4
Misses 24 24
|
@dantownsend Apologies for custom forms testing failing (I don't know why, but I can't run the Playwright tests locally as I have to upgrade my OS over the weekend, some important libraries are currently missing on my local machine and I can't run the tests). |
@sinisaos It's OK - I can run the tests locally. You can download the artefacts from GitHub actions which might provide some clues: https://github.com/piccolo-orm/piccolo_admin/actions/runs/4321016381 |
@dantownsend Once again, the Playwright proved to be very useful. I changed a lot of code to pass the test with custom forms because there was a conflict in the schemas. I tried using your approach. Can you check it when you have time? |
@sinisaos Yeah, the Playwright tests are great - I'd like to write more. I will add some for nullable columns soon. |
@dantownsend You don't even have to write tests, you can generate tests with playwright codegen and then copy-paste the generated code. |
@sinisaos I don't mind the auto generated tests. When there are lots of tests I like to follow this approach: |
This looks great. I just need to test it locally. |
Ideally this would be verified on the API level too, but could maybe implement that later on in |
I've tested it. I noticed a couple of things. Array columns still let you add or remove items: Read only fields only work on the edit page. So this is a tricky one ... should they also be read only when adding a new row? This means the value will be the default. I think it's useful for columns like the primary key, created on / modified on etc. |
@dantownsend I added a read-only to field label and disabled links for
That was my intention because it was required in the #271. You can easily enable read-only mode in add form as well by removing |
This PR has been marked as stale because it has been open for 30 days with no activity. Are there any blockers, or should this be closed? |
This PR has been marked as stale because it has been open for 30 days with no activity. Are there any blockers, or should this be closed? |
Related to #138 and partially to #271