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

Add docs on how to customize the structure updater pattern #785

Merged
merged 2 commits into from
Jul 13, 2017

Conversation

thet
Copy link
Member

@thet thet commented Jun 29, 2017

Here is some documentation on how to customize and overload the structure updater pattern.
It also applies on how to customize any pattern in general.

I've put it into mockup/mockup/patterns/structure - which doesn't follow our conventions but is probably a good place to put it to.
It's also written as .rst instead of MarkDown like the rest of mockups docs are written.
I used .rst because I know it better and I need some feedback anyways.

I'm not sure if this docs should live here or go to docs.plone.org - but I couldn't find an appropriate place there and I do not want to start a new chapter just to be able to put this documentation there....

any opinions or comments?

Copy link
Member

@agitator agitator left a comment

Choose a reason for hiding this comment

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

@thet could you add a example for the registry entry for titleSelector and descriptionSelector
otherwise lgtm

@thet
Copy link
Member Author

thet commented Jun 29, 2017

@agitator please re-review.

@thet thet requested a review from agitator June 29, 2017 13:25
Copy link
Member

@agitator agitator left a comment

Choose a reason for hiding this comment

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

lgtm


If you need some more control, you can overload the pattern and provide your own.

The pattern is registered in the RequireJS configuratio in ``mockup/mockup/js/config.js`` under the name ``mockup-patterns-structureupdater`` and under the path ``patterns/structure/pattern-structureupdater``.
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

typo: configuration (missing an "n")

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

the config.js path is mockup/js/config.js, right? mockup is there twice. One can assume that you mean inside the repository.


If you need some more control, you can overload the pattern and provide your own.

The pattern is registered in the RequireJS configuratio in ``mockup/mockup/js/config.js`` under the name ``mockup-patterns-structureupdater`` and under the path ``patterns/structure/pattern-structureupdater``.
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Typos:

  • configuration (misses the last n)
  • config.js path should have only one mockup in it, right? the first mockup would be the repository, but if one looks at this, already inside mockup, would think that there is two nested folders named mockup in mockup's repository (or at least that's what happened to me)

Copy link
Sponsor Member

@gforcada gforcada left a comment

Choose a reason for hiding this comment

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

See the inline feedback

@thet thet force-pushed the thet-structureupdater-docs branch from cc7ac09 to 03c6c02 Compare June 30, 2017 08:57
@thet
Copy link
Member Author

thet commented Jun 30, 2017

@gforcada done! plz re-review.
What's your opinion on place and format of this docs?

@thet thet requested a review from gforcada June 30, 2017 08:58
@gforcada
Copy link
Sponsor Member

gforcada commented Jul 3, 2017

@thet as for the text, it looks fine for me.

Regarding where it is placed, I don't see any other rst but the ones at the top level, all docs have been so far kept bundled inside the JS themselves. Is there a good reason to keep this one outside?

@thet
Copy link
Member Author

thet commented Jul 3, 2017

The reason I placed it within the structure pattern folder was, that wouldn't be a bad place if we hadn't already some kind of different structure and most importantly I wanted to get it done instead of thinking too long about the problem where to place it.

I should probably transfer it to MarkDown and place it alongside DEVELOP.md and LEARN.md.
I think it doesn't fit well into the autogenerated Mockup patterns overview page, as the text is too long.

@gforcada
Copy link
Sponsor Member

gforcada commented Jul 3, 2017

@thet I wouldn't block the merge due to where to place the docs, but maybe it would be a good idea to create an issue for that and discuss it there. Probably with the @plone/documentation-team involved as well 👍

@thet thet force-pushed the thet-structureupdater-docs branch from 03c6c02 to 3887447 Compare July 11, 2017 11:19
@thet thet force-pushed the thet-structureupdater-docs branch from 3887447 to 1b6e665 Compare July 11, 2017 11:21
@thet thet merged commit 69c3cc7 into master Jul 13, 2017
@thet thet deleted the thet-structureupdater-docs branch July 13, 2017 12:22
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.

3 participants