Jon, +1 for leaving the changeset as it is.
For future refactoring, all three options sound reasonable to me. For option 3, I think I would prefer a new subtype such as ListContent rather than doing it behind the scenes. Hannes > Am 13.04.2020 um 18:28 schrieb Jonathan Gibbons <[email protected]>: > > It's a valid comment, and something I have been considering as well. I > partially touched on this in some of earlier RFR notes. > > At some level, it's the pendulum swinging to the opposite extreme from the > poorly-named overused getMemberTree-style methods that used blockList for > everything. > > There are various ways forward, preferably in separate changesets. > > 1. Leave as-is, with multiple like-named methods, with the same impl. This > gives us the flexibility we need, if we ever want it, to set a different > `class` attribute on different impls. > > 2. Collapse the `getXyzList` and `getXyzListItem` methods into just two, > `getList` and `getListItem`, where the `getList` method and possibly the > `getListItem` methods take a parameter for a new enum type, `ListKind`. > > 3. Have the `getXyzList` methods return a different type (`ListContent`?) > such that `add`ing a method to these objects will automatically wrap the item > in a suitable container, which may be either a simple `<li>` element, or an > `<li>` element with an appropriate class attribute. > > 3a. Like 3, but without an explicit new type of Content ... just a new impl > behind the scenes, such as special subtype of HtmlTree for UL/OL elements. > > 3b. Some combination of 1, 2 and 3a. Either use separate `getXyzList` methods > or `getList(ListKind)`, but collapse the `getXyzListItem` methods into one, > and leave it up to the list object to decide whether to "customize" > individual list items returned from the general getListItem method. > > Whether to put a `class` attribute on a `<li>` element depends on whether we > want, or may want in the future, to use BEM naming for our CSS classes. In > full BEM, every HTML element gets its own `class` attribute with an > appropriate name. Right now, my sense is to /not/ use full BEM naming when > there is an inherent HTML relationship, such that we can use the `>` > selector, so that we can style the list items with `ul.LIST-CLASS > li { > STYLES }` > > --- > > For now, I'd like to leave this as an open/active discussion. Although there > are more uses of `<ul class="block-list">` on other pages, we have addressed > the 3 primary uses on the type-declaration pages. I'd like to continue the > cleanup for other parts of module/package/type declaration pages, and see > what other cleanup we would like to do. For example, I'd like to consider a > impl-method naming cleanup to use `newXyz` or `buildXyz` instead of `getXyz` > ... I'm thinking `newXyz` for basic factory methods, and `buildXyz` for more > complex constructions. I'd also like to move away from the `addXyz(Content > container)` style methods in which we build something and add it to a > container in a single call, and move towards `container.add(buildXyz())` as > providing a better separation of abstractions. > > -- Jon >
