Skip to content
This repository was archived by the owner on Apr 20, 2024. It is now read-only.

Feature/add some flexibility #12

Merged
merged 20 commits into from
Jun 5, 2018
Merged

Conversation

steffendsommer
Copy link
Contributor

@steffendsommer steffendsommer commented Jun 1, 2018

  • Use Request over Worker to have a db connection.
  • Allow for multiple async validators (could be turned into a protocol).
  • Allow for values to be accepted when isRequired is false (e.g. empty string).
  • Add a tag for generating input fields using Bootstrap tags for generating form elements (could be a starting point for Checkboxes #5).
  • Allow for setting custom view paths for form groups (solves Allow bootstrap size classing #8).

@codecov
Copy link

codecov bot commented Jun 1, 2018

Codecov Report

Merging #12 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@         Coverage Diff          @@
##           master   #12   +/-   ##
====================================
  Coverage       0%    0%           
====================================
  Files           8    13    +5     
  Lines         165   224   +59     
====================================
- Misses        165   224   +59
Impacted Files Coverage Δ
...ources/Submissions/SubmissionValidationError.swift 0% <ø> (ø) ⬆️
Sources/Submissions/SubmissionType.swift 0% <0%> (ø) ⬆️
Sources/Submissions/Tags/SubmissionsData.swift 0% <0%> (ø)
Sources/Submissions/SubmissionsProvider.swift 0% <0%> (ø) ⬆️
Sources/Submissions/FIeldCache.swift 0% <0%> (ø) ⬆️
Sources/Submissions/AbsentValueStrategy.swift 0% <0%> (ø)
Sources/Submissions/Submittable.swift 0% <0%> (ø) ⬆️
Sources/Submissions/FieldEntry.swift 0% <0%> (ø) ⬆️
...ources/Submissions/Configs/SubmissionsConfig.swift 0% <0%> (ø)
Sources/Submissions/AnyField.swift 0% <0%> (ø)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cdadfc2...e7a9b72. Read the comment docs.

@grosch
Copy link

grosch commented Jun 2, 2018

This is very cool. The three leaf files are all exactly the same, from a quick glance, except for the type of the input. Suggest instead we create an input-field.leaf file that gets included and uses a #get(fieldType) and then those other 3 files can all just set the type and then use #embed

@siemensikkema
Copy link
Contributor

@grosch at the moment they are the same. We will probably use 1 instead. We could create a generic input-field.leaf but we were not sure if we might want to specialize the different leaf files beyond just the input type.

README.md Outdated
@@ -32,6 +32,8 @@ targets: [
]
```

Next, copy/paste the `Resources/Views/Submissions` folder into your project in order to be able to use the provided Leaf tags.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to make it extra clear that these leaf files can be customized?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is explained later in the docs, but I've added a link to it and a recommendation on how to go about it.

README.md Outdated
@@ -276,14 +279,16 @@ and in `routes.swift` we'll add:
router.get ("todos/create", use: frontendTodoController.renderCreate)
```

> Note how we're using the `privateContainer` on the `Request` to limit the scope of our form validation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds a bit vague. How about something like "... on the Request since that is where the field cache is registered. Which is done to ensure the field cache does not outlive the request."

@@ -0,0 +1,19 @@
<div class="form-group">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we leave these duplicated leaf files for now? Or create one and point all of the config to that file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created one for now and then let's see if we need to split in the future.

@@ -0,0 +1,32 @@
import TemplateKit

public extension TagContext {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API docs

let hasErrors: Bool
}

public func submissionsData() throws -> SubmissionsData {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API docs

/// any validation errors.
/// - Returns: A `Future` of the `SubmissionType` value.
public func validate(inContext context: ValidationContext, on container: Container) -> Future<T> {
public func validate(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forget, did I have the order different here? Now it reads as if you're validating the submittable, which it not the case. The submittable is only for reference.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, updated to with submittable and moved it to be the second argument.

@steffendsommer steffendsommer merged commit cb13973 into master Jun 5, 2018
@steffendsommer steffendsommer deleted the feature/add-some-flexibility branch June 5, 2018 11:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants