search

frctl / fractalite

A prototype to help explore development ideas for future Fractal versions 🛠
MIT License
26 stars 1 forks source link

Initial short term feedback #3

Open samuelgoddard opened 5 years ago

samuelgoddard commented 5 years ago

High level feedback:

Absolutely love it, I've only really had time with the Nunjucks demo, but I feel like i'm going to miss some of the features from this already going back to current Fractal. I think you've smashed it and basically have nothing bad to say at all. I've noted down some random thoughts and feedback, let me know if anything doesn't make sense. Some high level Standout things:

More in depth feedback

Config / Preview

I think the project config file is way cleaner and easier to digest than before.

Really love the new ability to skeleton out your own navigation, super convenient and often requested.

I really like the new wrap ability especially how you can set this globally AND per component, it took me a litte while to get to grips with the docs for this but now I understand it i'm really into it. I also at first really wasn't a fan of not actually having a preview.html file, however I can now see why and think this is really great.

Scenarios

As mentioned I don't think I'm completely sold on the new name, my main feedback for this would be that the previous 'variant' naming lent itself way more to something like a 'primary' or 'secondary' button and directly correlated to something like a 'modifier' in BEM world. I can see how something like 'scenario' is way less restrictive though, just thought i'd give my initial thoughts.

Moving away from the name, I really like the new direction, rendering multiple scenarious per instance is my favourite addition, it's a quick and easy way to test out / show different content without having to make a weird pseudo variant. As mentioned I also love the name change from context to props, definitely sold on that. It'd be really cool if we could do Vue style required checks for component props, or optionally be able to pass in an object etc, rather than just a string.

One thing that slightly confused me in the docs and within the nunjucks example, with a component do you not now set a 'base' component? In the example the Button component has 2 scenarios, next and prev, however there's no default 'Button' props - in adapter integration section it explains it includes support for rendering sub-components (https://github.com/frctl/fractalite#adapter-integration) - however, the example doesn't actually have a base button config so it's a little confusing.

View templates

Not too much to say here, like the change to having a 'view' file, feels like a nicer split between view and data.

Pages

Really into this change, I personally think pages felt a little cumbersome before and removing the markdown requirement is a really good change IMO. Reference tags feel like a really nice upgrade from the previous methods too.

One question I had was this part of the docs (https://github.com/frctl/fractalite#nunjucks-templates) epxlained how you have access to the current compiler state, this is awesome, I was wondering if there's any plans to potentially be more granular with the state, as in, how hard would it be to do something like:

{% for component in components.units %}
{% endfor %}

to just loop over the components within the 01-units folder. That would be a really awesome feature and make building 'kitchen sink' style overview pages super easy.

Application UI

Not too much to say here, as peviously mentioned, the updates to the nav config are very welcome and super robust, really like this. The application feels clean out the box and as mentioned feels really snappy. Like the new cleaner out the box styles.

Themeing

Again, very welcome addition, I always found the current themeing method a bit of a pain and this feels lots nicer. Some feedback regarding the theme variables, it'd be nice if these where documented somewhere as you currenty have to find them in the theme style file. Obviously this is something that's probably already apparent, just thought it was worth mentioning. Didn't have time to dig into template overrides, but the method of configuring them seems way way easier than before.

Plugins

New plugins seem great, didn't have too much time to dig into this but from what i've seen, being able to easily create your own panels is something that's been requested a bunch of times so think that'll definitely be a hit.

The asset bundler plugin is a nice addition, quite often we have people asking about asset compilation for Fractal and don't immediately realise there isn't anything currently out the box for this, I think this will be a hugely welcome addition.

allmarkedup commented 5 years ago

Thanks again @samuelgoddard for this extensive feedback, really much appreciated. I'll try to answer your queries below...

The search is absolutely 10/10, hugely needed addition IMO and so nice to have this out the box, especially with per component hiding / aliases - is the plan to make this expandable? Just thinking it'd be great to optionally plug something like docsearch in.

Tbh I haven't really put much thought into making search pluggable/expandable yet - I guess I have been thinking of it more as a 'navigation shortcut' rather than as a full-blown search. Having said that, being able to swap out search implementations to support something a bit more extensive would be a very nice feature. I'll add that to the list of things to have a bit more of a think about!

Love the new info panel, I think the information provided is great and the new ability to see file sizes and skip between 'scenarios' quickly is a nice touch.

Great - this panel is still definitely work in progress as I'd like to allow plugins to add tables/content into this panel too.

I really like the new wrap ability especially how you can set this globally AND per component, it took me a litte while to get to grips with the docs for this but now I understand it i'm really into it. I also at first really wasn't a fan of not actually having a preview.html file, however I can now see why and think this is really great.

Excellent - I suspect most people don't really need (or want to have to deal with) the whole concept of preview templates and the idea is to make it easier for the majority of users to get up and running quickly. But I think that this should still give even the power users enough control (and even more flexibility in fact), albeit perhaps at the cost of a little more effort. And actually even replicating something like v1's file-based previews is in theory possible with plugins.

As mentioned I don't think I'm completely sold on the new name, my main feedback for this would be that the previous 'variant' naming lent itself way more to something like a 'primary' or 'secondary' button and directly correlated to something like a 'modifier' in BEM world.

Yeah I have gone back and forth on this one. The main problem with 'variants' is that a lot of people end up using them to test/develop things like 'button with long text' as well as things like 'primary button'. The former is really just an example of a button (actually probably like what you called a 'pseudo variant'), whereas the latter is a true 'variant'. But even a variant is really just a component template rendered with a set of default props, and I felt that 'scenario' is a better catch-all term for how people have been using them. It's certainly not set in stone yet however - if people feel strongly enough it's straightforward to switch it back to 'variants' :-)

It'd be really cool if we could do Vue style required checks for component props, or optionally be able to pass in an object etc, rather than just a string.

Absolutely - actually high up on the 'plugins I'd like to implement' list is one that allows components to specify a JSON schema for their props in their config that would then allow validation and even auto-generation of scenario data using something like JSON schema faker. Stuff like this is why I'm super excited about the whole plugin model.

One thing that slightly confused me in the docs and within the nunjucks example, with a component do you not now set a 'base' component?

No, basically the whole concept of 'base' components has gone away in the current implementation. To render an instance of a component you need to either specify a scenario or pass in a custom set of props (or both). Merging context data in v1 has been a massive cause of headaches and I wanted to avoid the situation of having to always override/merge with the 'base' set of context data/props. However, it'd be pretty easy to write a plugin that allows specifying of 'base' component props and that auto-merges all scenario props in with it - I'd just rather this be an explicit user decision rather than the default way of doing things. I hope that makes sense?!

I was wondering if there's any plans to potentially be more granular with the state, as in, how hard would it be to do something like: {% for component in components.units %}{% endfor %}

So I don't think the state would be more granular but that could easily be done by adding custom Nunjucks filters to the app - it might even be worth adding one or two extra ones into the default set available tbh. so you'd be able to do something like:

{% for component in components | filterByPath('units') %}
{% endfor %}

Or something like that. It means the state itself doesn't need any more logic but the templates can implement whatever they like. Plugins have full access to the app instance so can add custom template filters like this easily via the app.addViewFilter() method (the page rendering is done by the same nunjucks instance as all the other non-clientside rendered appliciation views).

Some feedback regarding the theme variables, it'd be nice if these where documented somewhere as you currenty have to find them in the theme style file.

Yes definitely! The whole theme styling (and variable naming) is very much WIP at the moment but once this moves along further the docs should definitely contain a full list of variable names.

The asset bundler plugin is a nice addition, quite often we have people asking about asset compilation for Fractal and don't immediately realise there isn't anything currently out the box for this, I think this will be a hugely welcome addition.

Yes I think this is probably one of the main things that trips people up with Fractal. I certainly never want it to be a requirement but it'll hopefully make the whole Fractal experience a lot more plug+play for new users.

Hopefully all that makes sense - if not (or if you disagree with anything) just shout, it's very early days yet with this and I'm very open to changes (for instance with renaming things) if consensus goes against the choices I've made. I get a bit stuck in my own head sometimes and outside perspective is always very welcome :-)