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

tv-override-missing-links: fix links to missing tiddlers #3530

Merged
merged 15 commits into from
Nov 24, 2018

Conversation

BurningTreeC
Copy link
Contributor

@BurningTreeC BurningTreeC commented Nov 10, 2018

this adds a variable tv-override-missing-links that gets detected by the link widget

that enables us to render links like in the Filter dropdown of the Advanced Search , even if enable links to missing tiddlers is unchecked in the Control-Panel-Settings-Tab

See discussion in the groups: https://groups.google.com/forum/#!topic/TiddlyWiki/E7qL1-hOOxM

fixes:

  • filter dropdown in AdvancedSearch
  • type dropdown in EditTemplate
  • field-name dropdown in EditTemplate

BurningTreeC added 7 commits November 10, 2018 08:12
this lets us set `tv-override-missing-links` true so that we can fix edge cases like the `Filter` dropdown in the `Advanced Search` when `enable missing links` is unchecked in the `Settings` tab of the Control Panel
@Jermolene
Copy link
Member

Hi @BurningTreeC this is a really interesting one. I'm afraid that merging the original PR #2347 was a mistake; it's what happens if I let my guard down and merge PRs without properly thinking them through. (To be clear, the fault here is entirely mine: it's my responsibility to act as a gatekeeper and not blindly wave things through).

It should never have been a global setting, but instead should have always been controlled by a variable tv-hide-missing-links, just like a bunch of other settings of the link widget. Then right at the top of the page template we can assign that variable from transcluding $:/config/MissingLinks. And then, just as you suggest, we'd need to wrap certain components with tv-hide-missing-links=no.

@BurningTreeC
Copy link
Contributor Author

@Jermolene is this fix here that much worse in terms of performance?

@Jermolene
Copy link
Member

is this fix here that much worse in terms of performance?

The fix here is more complex than it needs to be because of the earlier mistake. That contributes to making the core code harder to follow and understand, so I'd be inlined to clean things up properly.

@BurningTreeC
Copy link
Contributor Author

I understand - what about backwards compatibility @Jermolene? Here I'd be much more concerned than in #3572

@BurningTreeC
Copy link
Contributor Author

Now the variable used is tv-hide-missing-links="no"

@Jermolene
Copy link
Member

I understand - what about backwards compatibility @Jermolene? Here I'd be much more concerned than in #3572

I think the semantics of the revised implementation I'm proposing wouldn't introduce any problems; there would still be support for $:/config/MissingLinks but the problem areas would be fixed, and there'd be a new config variable.

BurningTreeC added 3 commits November 24, 2018 13:54
@BurningTreeC
Copy link
Contributor Author

@Jermolene , I've updated the PR ... I think this is similar to how you wanted it to be ... ?

@Jermolene
Copy link
Member

Great stuff, thank you @BurningTreeC!

@Jermolene Jermolene merged commit 3f91d5b into TiddlyWiki:master Nov 24, 2018
@BurningTreeC
Copy link
Contributor Author

@Jermolene, is it correct that the link widget doesn't have to refresh anymore if $:/config/MissingLinks changes? because I removed that

@Jermolene
Copy link
Member

@Jermolene, is it correct that the link widget doesn't have to refresh anymore if $:/config/MissingLinks changes? because I removed that

Yes, that's right. The set widget in the page template will ensure that the entire page gets refreshed if $:/config/MissingLinks changes

@BurningTreeC
Copy link
Contributor Author

Perfect, thanks!

@BurningTreeC BurningTreeC deleted the patch-52 branch November 24, 2018 13:41
@Jermolene
Copy link
Member

Whoops, I just started updating the docs and realised that we may have inadvertently flipped the logic of the $:/config/MissingLinks variable. Before this PR, setting that config tiddler to "no" would cause missing links to be hidden. Afterwards, setting it to "yes" will cause missing links to be hidden.

So, we need to change the variable to "tv-show-missing-links", and default it to "yes" if its not defined.

@BurningTreeC
Copy link
Contributor Author

Ups, my mistake! I didn't think about it at all

jho1965us pushed a commit to jho1965us/TiddlyWiki5 that referenced this pull request Apr 3, 2019
…yWiki#3530)

* add tv-override-missing-links variable

this lets us set `tv-override-missing-links` true so that we can fix edge cases like the `Filter` dropdown in the `Advanced Search` when `enable missing links` is unchecked in the `Settings` tab of the Control Panel

* add tv-override-missing-links to filter dropdown

* add tv-override-missing-links to type dropdown

* add tv-override-missing-links to fieldname dropd

* add tv-override-missing-links to TagManager(icons)

* undo tv-override-missing-links TagManager

not needed here

* Update link.js

* Update dropdown.tid

* Update fields.tid

* Update type.tid

* Update dropdown.tid

* Update link.js

* simplify all together

* add tv-hide-missing-links to pagetemplate

* do we need to refresh here...

... if the variable gets set on top of the pagetemplate?
jho1965us pushed a commit to jho1965us/TiddlyWiki5 that referenced this pull request Apr 3, 2019
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.

2 participants