Skip to content
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

Incompatibility with anlutro/laravel-settings #8

Closed
hubertnnn opened this issue Mar 11, 2019 · 2 comments
Closed

Incompatibility with anlutro/laravel-settings #8

hubertnnn opened this issue Mar 11, 2019 · 2 comments

Comments

@hubertnnn
Copy link

Since this package looks like a perfect pair for anlutro/laravel-settings it would be nice to make them work together.

The issue is that both packages use a common config option: settings.path.
Anluro is providing it as a full path ('path' => storage_path().'/app/settings.json',).
Your package is using just a subfolder path ('path' => 'app/settings.json',).

Since laravel itself resolves paths in config files (eg. cache.php has 'path' => storage_path('framework/cache/data'),) I think this package should have its config changed to reach compatibility.

As a bonus it will make it possible to put config in more places than just storage if needed.

@bakerkretzmar
Copy link
Owner

bakerkretzmar commented Mar 16, 2019

Thanks for this! I didn't realize I could resolve paths inside the config file, is there any benefit to doing it that way? My package does use storage_path() to resolve whatever path is passed in from the config.

To make this package compatible with anlutro's, I would have to actually change the name of the published config file, right? That would be a breaking change so I don't think I can promise it any time soon. In hindsight I probably should have chosen something a little more specific than 'settings' as a config file name, so I'll look at fixing that if I release another major version.

bakerkretzmar added a commit that referenced this issue Sep 29, 2019
@bakerkretzmar
Copy link
Owner

This should be resolved in v1.

bakerkretzmar added a commit that referenced this issue Sep 29, 2019
* Update JS dependencies

* Wip

* Set up javascript tests

* Some refactoring and some tests

* Improve controller testing

* Refactor and test Label

* Update Toggle and related test

* Refactor and add test for Text field

* Wip

* Extract shared styles into Nova-like wrapper

* Extract DefaultSetting layout component and apply to TextSetting

* Update partials and tests

* Rename TextSetting and remove File for now

* Rename config file to `nova-settings-tool`

Technically, fixes #8

* Refactor reading of settings and related tests

* Change package namespace and misc. wip

* Begin refactoring components and related tests

* Update TextareaSetting

* Update ToggleSetting

* Fix toggle setting and key SettingsController response by setting keys

* Rename ambiguous `default` to `placeholder`

* Update writing settings

* Update Code setting and tests

* Capitalize fallback setting labels

* Re-implement panels

* Readme and config updates

* Wip

* Remove File setting for now

* Implement Number setting

* Implement Select setting

* Allow loading settings file from any disk

* Set up GitHub actions

* Assets

* Apply fixes from StyleCI

* Wip

* Wip

* Wip

* Fix default config file

* New screenshot

* Wip
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants