Showing * for required Columns

[sadakchap]

Differentiating between required columns & non-required columns using *.

[mtrezza]

Thanks for opening this PR.

I wonder if we can find something visually more appealing than a text asterisk? We already have the field type next to it in a more subtile design, maybe we add something in that same fading gray after the type?

However, before getting into design, we should think about why we want this indication in the column name and what its purpose should be.

• When adding a row, the required fields are indicated as they are filled out.
• "Required" is not the only property of a field, it may also be "unique", "auto-filled", "with default value", etc. So why is it so important that of all properties we want to add the "required" flag to the column?

[sadakchap]

I wonder if we can find something visually more appealing than a text asterisk? We already have the field type next to it in a more subtile design, maybe we add something in that same fading gray after the type?

Yes, we can add that in fading grey. It will look like this.

But, for long column names it won't be visible at all.

• When adding a row, the required fields are indicated as they are filled out.

In master branch, for adding a new row we need to click on "Add" button. And if there are more than 2 or 3 required field and user does not know about it, then in worst user will have to click on "Add" button every time just to get field is required error.

• "Required" is not the only property of a field, it may also be "unique", "auto-filled", "with default value", etc. So why is it so important that of all properties we want to add the "required" flag to the column?

Flag only for required, as when user will edit or add new row, will have knowledge about all required columns, so that user does not have to wait for error note on every field. Specially when adding a new row and user has 5 or 7 required columns.

[mtrezza]

I think the asterisk is rather ambiguous, it's not clear what it means. In a form field (like the modal view to add a row) a user would assume that an asterisk means required, but not so much for a column name. It doesn't seem to be a usual UI concept.

Also, making it permanently visible seems unnecessary as it's always the same fields. The user will know after a few times which fields are required.

if there are more than 2 or 3 required field and user does not know about it, then in worst user will have to click on "Add" button every time just to get field is required error.

The solution to that would be to mark all required and missing fields at once. Can't we infer from the schema which fields are required, even before sending it to the server?

[sadakchap]

Also, making it permanently visible seems unnecessary as it's always the same fields. The user will know after a few times which fields are required.

True.

The solution to that would be to mark all required and missing fields at once. Can't we infer from the schema which fields are required, even before sending it to the server?

yes, we can get info about fields that are required from the schema. Could you please explain more about mark all required and missing fields at once. I mean how do we exactly want to mark all the fields & where?

[mtrezza]

Could you please explain more about mark all required and missing fields at once. I mean how do we exactly want to mark all the fields & where?

In #1692 we are displaying information about the cell instead of just (undefined). So for required fields that are empty, we could just display (required). This way a user can see without submitting which fields they need to fill out.

If a user submits a row with missing required fields anyway, an error message could be displayed and all fields that are required could have a red border (instead of blue), or however this is indicated currently. This all before submitting anything to the server.

[sadakchap]

something like this while adding new row? It does sound like a good idea. firstName is required field

But, for a field like password whose value is hidden & required, should we show only (hidden) or something else

[mtrezza]

But, for a field like password whose value is hidden & required, should we show only (hidden) or something else

How about (required) when it's empty and (hidden) when a value is set?

I think revealing whether any value is set or not should not have a significant security impact, because for existing rows, all pw fields should show (hidden), except the ones manually added to the DB where the field is actually missing. However, such a change should be highlighted in the changelog.

I think we can see that there is a lot of room for improvement in general. Like adding an i(nfo) icon to each column and with a click one could see / edit all field properties. But that's obviously beyond this PR.

[mtrezza]

Kindly let me know when this is ready for review :)

[sadakchap]

@mtrezza , thanks for waiting. I think it's ready for a review.

[mtrezza]

Thanks for your patience, I had time to review this, the only change I would make is to conform to this:

How about (required) when it's empty and (hidden) when a value is set?

This is currently not the case; after entering a password, the field still says requried instead of hidden:

Screen.Recording.2021-06-21.at.22.20.51.mov

[sadakchap]

Thank you @mtrezza for review. I tried implementing the requested changes. Please have a look and let me know if we need more changes 🙂

[mtrezza]

The password field now always show (hidden), even before entering any value. Is the password field always required, or can a user be created without setting a password, e.g. with authData only?

[sadakchap]

The password field now always show (hidden), even before entering any value.

I tried on latest 0989c0a of this #1720, and it did seem to me working fine.

Could you please try again after clearing cache & could you also please confirm the commit that you are trying on.

Is the password field always required, or can a user be created without setting a password, e.g. with authData only?

password is only required when username is provided.
Yes, user can be created without setting a password when authData is set.

[mtrezza]

Yes, user can be created without setting a password when authData is set.

So the field placeholders would have to change dynamically, is that even possible?

• The username, password and authData fields should show (required) initially, when none is being set.
• If authData has been set, then username and password should show (undefined), because they are not required anymore.
• If username has been set then password should show (required) and authData should show (undefined).

Does that make sense?

[sadakchap]

Thanks for patience @mtrezza, I tried making suggested changes, now it looks like this

requiredDiff.mp4

Please let me know about your thoughts

[mtrezza]

This is amazing! Just the password field still shows (hidden) even though no value has been set, so it should show (undefined). Is that possible?

[sadakchap]

Just the password field still shows (hidden) even though no value has been set, so it should show (undefined). Is that possible?

Is this a normal row(not a new row), if yes, then password field value will never come from backend & since this field is hidden, we are showing (hidden) for that.

else if it is a new row, I think It should show accordingly.

[mtrezza]

Do I read your comment correctly that the password field is never sent from the server, therefore the dashboard cannot know whether a value is set for password or not?

[sadakchap]

Do I read your comment correctly that the password field is never sent from the server, therefore the dashboard cannot know whether a value is set for password or not?

Yes.

const q = new Parse.Query('_User');
const res = q.find().then(res => console.log(res.map(o => o.id + ' password - ' + o.get('password'))))
// password will always be undefined

[mtrezza]

Then it makes sense to always display it as (hidden), even if it is actually (undefined). I think this PR should be ready, I'll just take a final look.

[mtrezza]

Just a tiny glitch: when deleting the username value, the password field does not change back from (undefined) to (required) with the red rectangle. Just as it changes immediately from (required) to (undefined) when entering a value for username, it should change back immediately when deleting the value for username.

Screen.Recording.2021-07-19.at.11.58.14.mov

[sadakchap]

Just a tiny glitch: when deleting the username value, the password field does not change back from (undefined) to (required) with the red rectangle. Just as it changes immediately from (required) to (undefined) when entering a value for username, it should change back immediately when deleting the value for username.

@mtrezza did you mean authData field does not change to (required) after deleting username field ?

[mtrezza]

Sorry, yes, authData, not password, as shown in the video.

[sadakchap]

@mtrezza can you please try with latest commit. I tired implementing requested changes

[mtrezza]

Looks good! Can you just resolve the merge conflicts and make sure this is based on master?

[sadakchap]

Can you just resolve the merge conflicts and make sure this is based on master?

Done!

[mtrezza]

Looks good for merge to me, thanks for this PR! We'll just wait for a second review approval to merge this.

[davimacedo]

LGTM!

[mtrezza]

@sadakchap Would you be interested in becoming an official member of the Parse Dashboard community? Your contributions over time have added great value to the community and further development of the product. We would be delighted to welcome you onboard. Please feel free to reach out for any questions you may have. You can respond here (in public) or send me a private message in the Parse Community Forum (@Manuel).

[sadakchap]

@sadakchap Would you be interested in becoming an official member of the Parse Dashboard community? Your contributions over time have added great value to the community and further development of the product. We would be delighted to welcome you onboard. Please feel free to reach out for any questions you may have. You can respond here (in public) or send me a private message in the Parse Community Forum (@Manuel).

😱 Sure, I would be very happy to become official member of Parse Dashboard community 😇. Thanks @mtrezza for reaching out to me.

[mtrezza]

Great, we will open a voting in the community forum in the coming days to confirm your membership. You can find more infos on how the process works in our Governance. Feel free to join the forum if you haven't already to follow the voting. There is no action required on your side at this point, but feel free to reach out anytime, preferably through the forum.

[RaschidJFR]

This feature is nice and useful, however, I'm finding it "over-restrictive" for some of the cases I commonly use. Example:

Let's say we have a class SmartContract with a required column abiInterface whose value is populated in the beforeSave trigger by calling a different microservice.

This new feature prevents running the beforeSave trigger to populate the required field which cannot be retrieved manually, so new records can't be added. A workaround would be to unmark the column as required, but that would remove the data integrity check.

Is it possible to do the validation server-side instead of client-side?

Another possible solution would be adding a default value (which could later be updated by the triggers), but they don't seem to work (they don't get loaded anywhere when adding a new record).

[mtrezza]

whose value is populated in the beforeSave trigger

You make a good point; I think we overlooked that scenario.

So you suggest the dashboard should only indicate that a field is required (which it does thanks to this PR), but not prevent from saving? It would be great if you could open a quick issue (and maybe even PR) for this, so we discuss there and track this.

[RaschidJFR]

Move the discussion to #1988