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

Feature: Typed Source #17

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Feature: Typed Source #17

wants to merge 15 commits into from

Conversation

LuanHimmlisch
Copy link
Member

This PR is meant to refactor the code relating to the state of the Front actions taken by the user, also known as the source type. Previously, this source was managed as a simple string that prevents Intellisense from working, and allows human error.

This PR does the following to fix the mentioned errors:

  • Adds a WeblaborMx\Front\Source class, that is meant to be used as an Enum compatible with PHP 7.1.
  • Adds constants to enumerate the different cases.
  • Adds various methods to check whether a source is certain case, with the added utilities of isForm(), isServerSide() and isClientSide().
  • Adds validations to ensure the values of the source are valid according to the constants.
  • Decouples the session() out of the Sourceable trait, in order to reuse it inside the Input class.
  • Makes the Source class Stringable so it can be cast to string and used as it was.
  • Marks the use of the $source variable as @deprecated to prevent the usage.

With this PR, the value of the source is always known, through enumeration, validation and IDE autocompletion.

@LuanHimmlisch
Copy link
Member Author

Notable code:

No magic strings, everything is enumerated.

image

Helper functions such as isForm.

image

Removed usage of $source directly. Using class methods instead.

image

This allows for different ways to structure the resources, instead of hiding the inputs. Specially useful for larger resources.

image

@skalero01
Copy link
Contributor

Please update the readme with the changes

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

Successfully merging this pull request may close these issues.

2 participants