Trim Saga: Another batch of medium-sized files #6270
Conversation
| <li> | ||
| <$list filter="[<__prefix__>addsuffix<__chunk__>is[shadow]] [<__prefix__>addsuffix<__chunk__>is[tiddler]]" variable="full-title"> | ||
| <$list filter="[<full-title>removeprefix<__prefix__>]" variable="chunk"> | ||
| <span>{{$:/core/images/file}}</span> <$macrocall $name="leaf-link" full-title=<<full-title>> chunk=<<chunk>>/> | ||
| <span>{{$:/core/images/file}}</span> <$macrocall $name="leaf-link" full-title=<<full-title>> chunk=<<chunk>>/> |
There was a problem hiding this comment.
There are some tc-xxx-gap classes, that will allow you to add CSS gaps left, right and center if needed. So "artificial" spaces are not needed.
TiddlyWiki5/themes/tiddlywiki/vanilla/base.tid
Line 3033 in d3ea98f
This gap here can be added to <$link to=<<__full-title__>>><$text text=<<__chunk__>>/></$link> in the leaf-link macro from line 4. Like so: <$link to=<<__full-title__>> class="tc-tiny-gap-left"><$text text=<<__chunk__>>/></$link>
The other elements may need an additional span for proper "spacing".
There was a problem hiding this comment.
Is that the actual design preference to use those classes rather than  ? Or, as I saw used all over the place, ? My aim when creating these PRs was to reduce changes to the resulting parse tree as much as possible, so a few deliberately placed spaces was better for that than introducing classes.
Is there a final decision on this? I was following Jeremolene's remarks from here, but I don't know if he remembered the classes at the time either. I hadn't known about them.
There was a problem hiding this comment.
The classes are not very old. They have been created, because whitespace shouldn't be used for styling. CSS should be used for styling. ...
IMO styled SPANs are semantically correct HTML code. Whitespace is not. We have a lot of these problems in the TW UI. They constantly cause problems, if someone wants to adjust or create new themes.
There was a problem hiding this comment.
Is this a change we want to make while I'm converting over everything to use \whitespace trim? Or should it be done separately? I would argue separately.
- The use of these classes is a distinct design decision that would be on top of switching over to using trim.
- It would be easier to see where we need these classes once we have whitespace trim and
 scattered about. - I'm having a hard enough time getting these changes merged as they are without compounding the number of judgement calls.
There was a problem hiding this comment.
OK. You are right. Let's make "baby steps" ;)
There was a problem hiding this comment.
I'm not convinced about the new spacing classes. It seems to me that using whitespace for certain purposes like separating controls from their text is perfectly reasonable because a unit of whitespace has a clear semantic meaning.
Using   is clever, but existing code tends to use <$text text=" "/>. I've no objection to a blanket change, but again it would require care. In the meantime, we should try to be consistent, or at least not make the inconsistencies worse.
There was a problem hiding this comment.
I'm OK if a space is used here, since it doesn't seem to cause problems, if users select and copy/paste content with an accidental white space.
The "gap" classes where invented, to separate tag-pill content. Eg: tag-icons where separated from tag-title-text with a space.
The "space" character caused problems if a tag-pill text was copy pasted. Most users accidentally also copied the space, which in turn caused problems if pasted into an other tag of a new tiddler. The new tag contained a leading space, which caused weird and hard to find problems.
There was a problem hiding this comment.
@Jermolene, So no worries about <$text text=" "/>. They have all already been removed as a part of #5473. The only one that remains is on inside the "menubar" plugin, and it is literally having no effect (It's not being used with \whitespace trim).
@pmario, I hadn't considered the copy-paste implications of those spaces. That does seem an argument for removing them in favor of spacing classes, but I still of the opinion that it should be a separate issue from the "\whitespace trim" additions.
There was a problem hiding this comment.
@flibbles You are right. I'm OK with   here. I don't think, there is a copy paste side effect here.
|
I personally would like to have this PR out of the way, because I would like to change the core tabs-macro. Every PR that I create will cause a conflict for your PR or for mine, if yours is merged first. |
I wouldn't let that stop you. I've touching nearly 20 files right now with my Trim stuff... and I don't know when or if any of it will merge in. Those 20 files can't be considered "locked". As long as you maintain the consistent use of "\whitespace trim", I can just resolve conflicts by merging in favor of your changes. |
|
Are these waiting on anything? I see we're approaching another release cycle 5.2.3. Sooner or later, these whitespace fixes are going to go stale. I'd say either merge 'em or close 'em. |
|
Ugh. That was a pain. Resolved a conflict on this. Required me to change my authtoken settings in github. |
|
DOUBLE UGH. Someone wrote tests for that tabs file SINCE I wrote this PR. Reconciled and ready to merge. |
|
Thanks @flibbles |
I'd tag these for V5.2.2 myself if I could. Stop me if all this is a hassle.
Prettied up EditTemplate a bit since we can now.