Example templates - Githubissues

BoltAuth / Auth

Auth extension for Bolt

MIT License

16 stars 17 forks source link

Example templates #41

Closed jadwigo closed 6 years ago

jadwigo commented 6 years ago

This PR does a lot in the templates

It removes most of the css framework specific code from the default templates, the framework used is not compatible with the new basetheme anyway - so removing it gives developers a clean place to start
All default configuration values are added to config.yml.dist as comments, so configuring the extension is easier
There are some comments and paths updated.
The navigation in the templates is updated so users can click through to their profile.
It also adds the twig function {{ auth_link_profile_view() }} so linking to the profile is easier in templates
The broken twig loop in templates/profile/edit.twig is repaired.

jadwigo commented 6 years ago

Obviously {{ auth_link_profile_view() }} has to be added to the docs too.

SvanteRichter commented 6 years ago

I'm not a fan of having commented configs in the config file (those belong in the docs), especially not if the value is null (those really belong in the docs), also there seems to be some inconsistency in the indentation in the templates, some of them indent html inside of blocks and some do not. Not sure if we have been consistent before, but if it's okay with you I prefer indented.

jadwigo commented 6 years ago

I see you mean with the commented configs. Ideally you would not need them, and personally I would also remove all unused options in my sites.

I feel that it helps developers discover options, especially if they configure the extension for the first time without knowledge of the overall structure. So it is not the "clean" option, but it helps people start using the extension, so from a usability point of view I would like to have the options visible in the config.

SvanteRichter commented 6 years ago

Please resolve the conflicts from merging your other PRs and I'll see about getting this in :)

jadwigo commented 6 years ago

Some rebasing seems to have worked.

jadwigo commented 6 years ago

pinging @SahAssar Is this ready to be merged?

SvanteRichter commented 6 years ago

LGTM, but I might think about making this bump a major since it changes so much in the templates. @jadwigo Does this need changes in the docs too?

jadwigo commented 6 years ago

Most of the docs won't need an update, but I'll go through the docs https://github.com/BoltAuth/Auth/issues/49 and update them where needed. Also the global feedback will need some docs and an example usage.