add read_only_columns to TableConfig

[sinisaos]

Related to #138 and partially to #271

[dantownsend]

@sinisaos Thanks for this.

It looks like we're hiding inputs if they're marked as readonly, but another option is to set the readonly attribute, so they are still visible, just not editable:

https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/readonly

You should be able to do this using Vue:

:readonly="isReadOnlyString(columnName)"

[sinisaos]

You should be able to do this using Vue:

:readonly="isReadOnlyString(columnName)"

@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 readonly attribute does not work and we have to use the disabled attribute and we would have to change a lot more code.

If you want I can make another commit to make the fields visible but readonly or disabled, so we can see what works best?

[sinisaos]

@dantownsend With the latest commit the field should be visible but read-only or disabled in the edit form. I hope this is better and I hope I have included all the possible fields.

[codecov-commenter]

Codecov Report

Merging #289 (bc063ba) into master (ba9da95) will increase coverage by 0.07%.
The diff coverage is 100.00%.

❗ 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              

Impacted Files	Coverage Δ
piccolo_admin/endpoints.py	92.70% <100.00%> (+0.08%)	⬆️

[sinisaos]

@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).

[dantownsend]

@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

[sinisaos]

@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?

[dantownsend]

@sinisaos Yeah, the Playwright tests are great - I'd like to write more. I will add some for nullable columns soon.

[sinisaos]

@dantownsend You don't even have to write tests, you can generate tests with playwright codegen and then copy-paste the generated code.

[dantownsend]

@sinisaos I don't mind the auto generated tests. When there are lots of tests I like to follow this approach:

https://playwright.dev/docs/pom

[dantownsend]

This looks great. I just need to test it locally.

[dantownsend]

Ideally this would be verified on the API level too, but could maybe implement that later on in PiccoloCRUD. For now, we might need to add something to the docs warning people that it's the case.

[dantownsend]

I would put the ReadOnlyColumns table also under the Testing group.

[dantownsend]

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]

Another suggestion - we could put read only next to the label, so it's obvious that it's read only. Otherwise users might think it's just broken.

You can add this CSS to show a special cursor too:

input:read-only {
    cursor: not-allowed !important;
}

[sinisaos]

@dantownsend I added a read-only to field label and disabled links for ArrayWidget in read-only mode.

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.

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.$route.params.rowID !== undefined from the isReadOnly method in RowForm.vue but I don't think it's necessary because the default value is given by default and you can change it or not, and read-only columns are usually to prevent data editing. Sorry if I missed your point, but feel free to change it, if you think that's better.

[github-actions]

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?

[github-actions]

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?