Replace Tails by turboprop #84
Conversation
Reviewer's Guide by SourceryThis pull request replaces the Tails dependency with a custom implementation borrowed from the Turboprop project for merging Tailwind CSS classes. The implementation includes a comprehensive class merging system with caching capabilities and support for various Tailwind features. No diagrams generated as the changes look simple and do not need a visual representation. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @bluzky - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Testing: 1 issue found
- 🟡 Complexity: 1 issue found
- 🟡 Documentation: 2 issues found
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
| # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | ||
| # SOFTWARE. | ||
|
|
There was a problem hiding this comment.
suggestion (testing): Consider adding tests for all new utility modules
I notice that while several utility modules are being added (class.ex, class_tree.ex, config.ex, parser.ex), there are only tests for merge and validator. Consider adding dedicated test files for each module to ensure complete coverage of the new functionality.
defmodule SaladUI.Utils.ClassTest do
use ExUnit.Case
alias SaladUI.Utils.Class
test "class utility functions" do
assert Class.join(["btn", "primary"]) == "btn primary"
assert Class.join(["btn", nil, "primary"]) == "btn primary"
end
end
|
|
||
|
|
||
| 4. **Add javascript to handle event from server** | ||
| 3. **Add javascript to handle event from server** |
There was a problem hiding this comment.
issue (documentation): Step numbering needs to be updated after removal of tails section
The steps go from 2 to 4, skipping 3. Please update the numbering to be sequential.
| #> mix salad.add label button | ||
| ``` | ||
|
|
||
| 2. **Using `salad_ui` as a library:** | ||
|
|
||
| - Adding `salad_ui` to your list of dependencies in `mix.exs`: | ||
|
|
||
| ```elixir | ||
| def deps do | ||
| [ | ||
| {:salad_ui, "~> 0.13.0", only: [:dev]} | ||
| ] | ||
| end | ||
| ``` | ||
|
|
||
| 3. **Using `salad_ui` as a library:** |
There was a problem hiding this comment.
suggestion (documentation): Installation methods could be more clearly distinguished
Consider adding a brief comparison of when to use each installation method at the start of the installation section to help users choose the appropriate approach.
## Installation
Choose your installation method:
- **As a library**: Use this if you want to use pre-built components without modifications
- **As a project dependency**: Use this if you need to customize components or use specific ones
2. **Using `salad_ui` as part of your project:**
> Install only the components you need and customize the source code to fit your requirements.
- Init Salad UI in your project
| end | ||
| end | ||
|
|
||
| def generate do |
There was a problem hiding this comment.
issue (complexity): Consider flattening the nested tree structure into a single-level map with compound keys
The nested tree structure with recursive operations can be simplified to a flat map while maintaining the same functionality. Consider this approach:
def generate do
config = Config.config()
theme = Map.get(config, :theme, %{})
prefix = Map.get(config, :prefix)
groups = Map.get(config, :groups, [])
classes =
groups
|> Enum.flat_map(fn {group, next} ->
flatten_classes(next, group, prefix)
end)
|> Map.new()
end
defp flatten_classes(next, group, prefix, path \\ "") do
Enum.flat_map(next, fn
{sub_group, sub_next} ->
new_path = join_path(path, sub_group)
flatten_classes(sub_next, group, prefix, new_path)
part when is_binary(part) ->
full_path = join_path(path, part)
[{full_path, %{"group" => group, "validators" => %{}}}]
part when is_function(part) ->
handle_function(part, group, path)
end)
endThis approach:
- Uses a flat map with compound keys instead of nested "next" structures
- Eliminates deep merges and complex recursion
- Maintains the same functionality with simpler code
- Makes the data structure easier to traverse and modify
The resulting data structure would be a single-level map with paths as keys, making it more efficient to look up and modify values.
|
Just noticed that we need to ass |
|
thanks @bmalum I'll update document |
Some forks report issue related to
tailsas discussed here: https://elixirforum.com/t/turboprop-toolkit-to-create-accessible-component-libraries/64228I decided to borrow
Mergemodule from Turboprop and copied to SaladUI code base. Sadly becauseTurbopropis now discontinued.I've tested on all existing components, and so far so good.
Summary by Sourcery
Replace 'tails' with 'turboprop' for merging Tailwind CSS classes, update related configurations, and refactor tests to improve maintainability.
Enhancements:
Documentation:
Tests:
Chores: