DOC Update js docs #130
Merged
DOC Update js docs #130
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Entwine is effectively a native part of silverstripe now - we should house its docs ourselves.
GuySartorelli
commented
Dec 20, 2022
| [Howto: Extend the CMS Interface](/developer_guides/customising_the_admin_interface/how_tos/extend_cms_interface). | ||
|
|
||
| ## Installation |
There was a problem hiding this comment.
Instead of updating this information I've removed it, since it doesn't belong in this section anyway.
| @@ -268,10 +232,6 @@ in jQuery.entwine, we're trying to reuse library code wherever possible. | |||
| The most prominent example of this is the usage of [jQuery UI](http://jqueryui.com) for | |||
| dialogs and buttons. | |||
|
|
|||
| The CMS includes the jQuery.entwine inspector. Press Ctrl+` ("backtick") to bring down the inspector. | |||
There was a problem hiding this comment.
This is just false.
Comment on lines
-317
to
+277
| `window.ss.routeRegister` wrapper. The `history` object is passed to react components. | ||
| `window.ss.routeRegister` wrapper. |
There was a problem hiding this comment.
That isn't passed down with the new version of react-router
Comment on lines
307
to
310
| Firstly, where the page.js router uses the full path (e.g. 'admin/pages') as its route, | ||
| react-router uses relatively paths with nested routes. The main route is, by default, 'admin', | ||
| which means the 'admin/pages' path would declare its route as 'pages'. This is added against the | ||
| `reactRoutePath` key in the array returned from `LeftAndMain::getClientConfig()`. |
There was a problem hiding this comment.
react-router now uses relative paths. The reactRoutePath was added to accomodate this.
| @@ -9,41 +9,27 @@ iconBrand: js | |||
| The following document is an advanced guide on building rich javascript interactions within the Silverstripe CMS and | |||
| a list of our best practices for contributing and modifying the core javascript framework. | |||
|
|
|||
| ## ES6 and build tools | |||
There was a problem hiding this comment.
This isn't really relevant for this section of docs, so it was easier to remove it than to update it.
Comment on lines
-62
to
-69
| `jQuery.fn` namespace, and attaches itself automatically to all jQuery collections. The basics for are outlined in the | ||
| official [jQuery Plugin Authoring](http://docs.jquery.com/Plugins/Authoring) documentation. | ||
|
|
||
| There a certain [documented patterns](http://www.learningjquery.com/2007/10/a-plugin-development-pattern) for plugin | ||
| development, most importantly: | ||
|
|
||
| * Claim only a single name in the jQuery namespace | ||
| * Accept an options argument to control plugin behavior |
There was a problem hiding this comment.
Our docs don't need to explain what jQuery Plugins are good for or how to build them. Leave that to the official docs.
Comment on lines
-135
to
-146
| See the [official developer guide](http://jqueryui.com/docs/Developer_Guide) and other | ||
| [tutorials](http://bililite.com/blog/understanding-jquery-ui-widgets-a-tutorial/) to get started. | ||
|
|
||
| Example: Highlighter | ||
|
|
||
| ```js | ||
| (function($) { | ||
| $.widget("ui.myHighlight", { | ||
| getBlink: function () { | ||
| return this._getData('blink'); | ||
| }, | ||
| setBlink: function (blink) { |
There was a problem hiding this comment.
Our docs don't need to explain what jQueryUI widgets are good for or how to build them. Leave that to the official docs.
|
|
||
| ### jQuery.Entwine | ||
|
|
||
| jQuery.entwine is a third-party plugin, from its documentation: | ||
| "A basic desire for jQuery programming is some sort of OO or other organisational method for code. For your |
There was a problem hiding this comment.
I couldn't find that quote - but we're including the entwine docs directly now anyway so it doesn't make sense to quote them like this.
Comment on lines
-346
to
-349
| See [interactive example on jsbin.com](http://jsbin.com/opuva) | ||
|
|
||
| You can also use the [jQuery.metadata Plugin](http://docs.jquery.com/Plugins/Metadata/metadata) to serialize data into | ||
| properties of DOM elements. This is useful if you want to encode element-specific data in markup, for example when |
There was a problem hiding this comment.
Our JSbin examples don't seem to be there anymore.
jQuery.metadata doesn't seem to be a thing anymore.
Comment on lines
-587
to
-594
| ### Unit Testing | ||
|
|
||
| It is important to verify that your code actually does what it says, and the best way to ensure this are **automated | ||
| tests**. For jQuery, we use two different tools with different uses: **unit testing** with | ||
| [QUnit](http://docs.jquery.com/QUnit) (also used by the jQuery team for the core libraries), and **behaviour driven | ||
| testing** with [JSpec](http://visionmedia.github.com/jspec/). There are overlaps between the two solutions, if in doubt | ||
| start with JSpec, as it provides a much more powerful testing framework. | ||
|
|
There was a problem hiding this comment.
This information was just flat out wrong. Easier to just remove it - people know to test their stuff.
5 tasks
GuySartorelli
force-pushed
the
pulls/5/update-js-doc
branch
from
9ea6b9b to
2fd6470
Compare
GuySartorelli
commented
Dec 20, 2022
|
|
||
| ```js | ||
| import { withRouter } from 'react-router'; |
There was a problem hiding this comment.
This HOC no longer exists in react-router. We do offer a replacement (outlined in the changelog) but it's a bit of a hack and should not be used for any new components. It's only there to facilitate having a clear upgrade path and is clearly marked as a "push the can down the road" solution.
emteknetnz
requested changes
Dec 21, 2022
en/04_Changelogs/5.0.0.md
Outdated
|
|
||
| ### Webpack config | ||
|
|
||
| The [@silverstripe/webpack-config](https://www.npmjs.com/package/@silverstripe/webpack-config) and [@silverstripe/eslint-config](https://www.npmjs.com/package/@silverstripe/eslint-config) NPM libraries, along with the build stack for all supported modules, has been updated to be compatible with node 18 and webpack 5. This will only impact you if your module or project uses one or both of those NPM packages - you will need to make sure you upfate your own dependencies to be compatible with them, along with the dependencies listed below if you use them. |
There was a problem hiding this comment.
Suggested change
| The [@silverstripe/webpack-config](https://www.npmjs.com/package/@silverstripe/webpack-config) and [@silverstripe/eslint-config](https://www.npmjs.com/package/@silverstripe/eslint-config) NPM libraries, along with the build stack for all supported modules, has been updated to be compatible with node 18 and webpack 5. This will only impact you if your module or project uses one or both of those NPM packages - you will need to make sure you upfate your own dependencies to be compatible with them, along with the dependencies listed below if you use them. | |
| The [@silverstripe/webpack-config](https://www.npmjs.com/package/@silverstripe/webpack-config) and [@silverstripe/eslint-config](https://www.npmjs.com/package/@silverstripe/eslint-config) NPM libraries, along with the build stack for all supported modules, has been updated to be compatible with node 18 and webpack 5. This will only impact you if your module or project uses one or both of those NPM packages - you will need to make sure you update your own dependencies to be compatible with them, along with the dependencies listed below if you use them. |
| Silverstripe CMS supports the following web browsers: | ||
| Silverstripe CMS uses [browserslist](https://browsersl.ist) default settings to determine which browsers are supported. Note that this only applies for the CMS itself - you can support whatever browsers you want to in the front end of your website. | ||
|
|
||
| These settings ensure support for the latest 2 versions of major browsers, plus all versions of those browsers with at least 0.5% worldwide marketshare, plus the [Firefox Extended Support Release](https://support.mozilla.org/en-US/kb/choosing-firefox-update-channel). They explicitly exclude browser versions which have reached end-of-life. |
There was a problem hiding this comment.
Suggested change
| These settings ensure support for the latest 2 versions of major browsers, plus all versions of those browsers with at least 0.5% worldwide marketshare, plus the [Firefox Extended Support Release](https://support.mozilla.org/en-US/kb/choosing-firefox-update-channel). They explicitly exclude browser versions which have reached end-of-life. | |
| These settings ensure support for the latest 2 versions of major browsers, plus all versions of those browsers with at least 0.5% worldwide market share, plus the [Firefox Extended Support Release](https://support.mozilla.org/en-US/kb/choosing-firefox-update-channel). They explicitly exclude browser versions which have reached end-of-life. |
Comment on lines
260
to
261
| Declaring a function with the name `onmatch` will create a behavior that is called on each object when it matches. Likewise, `onunmatch` will | ||
| be called when an object that did match this selector stops matching it (because it is removed, or because you've changed its properties). |
There was a problem hiding this comment.
Suggested change
| Declaring a function with the name `onmatch` will create a behavior that is called on each object when it matches. Likewise, `onunmatch` will | |
| be called when an object that did match this selector stops matching it (because it is removed, or because you've changed its properties). | |
| Declaring a function with the name `onmatch` will create a behavior that is called on each object when it matches. Likewise, `onunmatch` will be called when an object that did match this selector stops matching it (because it is removed, or because you've changed its properties). |
There was a problem hiding this comment.
I don't understand the need for this change, but it won't change anything so I'll do it lol
Comment on lines
+9
to
+13
| Some admin modules render their UI with React, a popular Javascript library created by Facebook. | ||
| For these sections, rendering happens via client side scripts that create and inject HTML | ||
| declaratively using data structures. | ||
|
|
||
| Even within sections that are _not_ primarily rendered in react, several React components may be injected into the DOM. |
There was a problem hiding this comment.
Suggested change
| Some admin modules render their UI with React, a popular Javascript library created by Facebook. | |
| For these sections, rendering happens via client side scripts that create and inject HTML | |
| declaratively using data structures. | |
| Even within sections that are _not_ primarily rendered in react, several React components may be injected into the DOM. | |
| Some admin modules render their UI with React, a popular Javascript library created by Facebook. For these sections, rendering happens via client side scripts that create and inject HTML declaratively using data structures. | |
| Even within sections that are _not_ primarily rendered in react, several React components may be injected into the DOM. |
There was a problem hiding this comment.
I don't understand the need for this change, but it won't change anything so I'll do it lol
|
|
||
| The recommended way to access these dependencies is by using the [@silverstripe/webpack-config npm package](https://www.npmjs.com/package/@silverstripe/webpack-config). The documentation in the readme for that package explains how to use it. | ||
|
|
||
| If you are not using webpack to transpile your javascript, see if your build tooling has an equivalent to webpack's `externals` configuration. Alternatively, instead of `import`ing these depdencies, you can access them on the `window` object (for example the injector module is exposed as `window.Injector`). |
There was a problem hiding this comment.
Suggested change
| If you are not using webpack to transpile your javascript, see if your build tooling has an equivalent to webpack's `externals` configuration. Alternatively, instead of `import`ing these depdencies, you can access them on the `window` object (for example the injector module is exposed as `window.Injector`). | |
| If you are not using webpack to transpile your javascript, see if your build tooling has an equivalent to webpack's `externals` configuration. Alternatively, instead of `import`ing these dependencies, you can access them on the `window` object (for example the injector module is exposed as `window.Injector`). |
Comment on lines
+138
to
+139
| To avoid transpiling these into your own generated bundles, | ||
| we have exposed many libraries as [Webpack externals](https://webpack.js.org/configuration/externals/). |
There was a problem hiding this comment.
Suggested change
| To avoid transpiling these into your own generated bundles, | |
| we have exposed many libraries as [Webpack externals](https://webpack.js.org/configuration/externals/). | |
| To avoid transpiling these into your own generated bundles, we have exposed many libraries as [Webpack externals](https://webpack.js.org/configuration/externals/). |
There was a problem hiding this comment.
I don't understand the need for this change, but it won't change anything so I'll do it lol
Comment on lines
+27
to
+30
| Because we use [Bootstrap 4](https://getbootstrap.com/) which | ||
| does not follow [BEM](http://getbem.com/) naming convention there will be | ||
| a lot of places where class names voilate BEM. | ||
| However, please note that they are not a indicator of how to name classes. |
There was a problem hiding this comment.
Suggested change
| Because we use [Bootstrap 4](https://getbootstrap.com/) which | |
| does not follow [BEM](http://getbem.com/) naming convention there will be | |
| a lot of places where class names voilate BEM. | |
| However, please note that they are not a indicator of how to name classes. | |
| Because we use [Bootstrap 4](https://getbootstrap.com/) which does not follow [BEM](http://getbem.com/) naming convention there will be a lot of places where class names voilate BEM. However, please note that they are not a indicator of how to name classes. |
There was a problem hiding this comment.
I don't understand the need for this change, but it won't change anything so I'll do it lol
There was a problem hiding this comment.
Actually I won't. Look at the file as a whole. Making this change here and not through the whole file will make it harder to maintain. Let's leave this sort of thing until we're ready to do #11
GuySartorelli
force-pushed
the
pulls/5/update-js-doc
branch
from
2fd6470 to
abc9ffc
Compare
A lot of functionality wasn't being documented. Also re-order some of the doc to be more logical, and remove stuff we don't need like the license which is included with the source code. This commit also takes the context of Silverstripe CMS into account, referencing several CMS-specific uses of entwine.
GuySartorelli
force-pushed
the
pulls/5/update-js-doc
branch
from
abc9ffc to
995b414
Compare
emteknetnz
approved these changes
Dec 21, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DO NOT SQUASH. These commits are separated to make it clear what's changed vs what's just copied
Parent issue