Settings icon indicating copy to clipboard operation
Settings copied to clipboard

Always overwrite Field name attribute to Value

Open bojmaliev opened this issue 2 years ago • 1 comments

WHY

In documentation it says: field (Backpack CRUD field configuration in JSON format. https://backpackforlaravel.com/docs/crud-fields#default-field-types)

As new to backpack, I lost an hour trying to figure it out Why my settings weren't working while I was setting field name to title or logo or whatever.

BEFORE - What was wrong? What was happening before this PR?

If somebody writes field name to something else than value, the update operation isn't working.

AFTER - What is happening after this PR?

We make sure always field name is set to value

How did you achieve that, in technical terms?

Always overwriting $field['name'] = 'value';

Is it a breaking change or non-breaking change?

Non-breaking change

How can we test the before & after?

??

bojmaliev avatar Apr 07 '22 18:04 bojmaliev

BOOM! Your first PR with us, thank you so much! Someone will take a look at it shortly.

Please keep in mind that:

  • if this constitutes a breaking change, it might take quite a while for this to get merged; we try to emulate the Laravel release cycle as much as possible, so developers can upgrade both software once; this means a new big release every ~6 months;
  • even if it's a non-breaking change, it might take a few days/weeks for the PR to get merged; unless it's a no-brainer, we like to have some community feedback on new features, before we merge them; this leads to higher-quality code, in the end; we learnt this the hard way :-)
  • not all PRs get merged; sometimes we just have to hold out new features, to keep the packages lean; sometimes we don't include features that only apply to niche use cases;
  • we're not perfect; if you think we're wrong, call us out on it; but in a kind way :-) we all make mistakes, best we learn from them and build better software together;

Thank you!

-- Justin Case The Backpack Robot

welcome[bot] avatar Apr 07 '22 18:04 welcome[bot]

Hey @bojmaliev sorry it took me so much time to get back here.

I don't think forcing the field name is the solution here. I think this is a documentation problem.

I've just added it here: https://github.com/Laravel-Backpack/Settings/pull/136

Hope no other developers need to waste time on this.

Sorry this one didn't get merged, thanks for your time 🙏

Cheers

pxpm avatar Apr 02 '24 15:04 pxpm