Make anchor links use UUID instead of title #247
Conversation
| @@ -412,7 +412,7 @@ class DocFragment extends Component { | |||
| </Flex> | |||
| )} | |||
| <h2> | |||
| <AnchorLink text={this.props.content[this.getTitleKey()]} /> | |||
| <AnchorLink text={this.props.content.getUuid()} /> | |||
There was a problem hiding this comment.
I guess something similar also ought to be done on line 429?
| text={ | ||
| this.props.content[this.getTitleKey()] + '-' + title | ||
| } | ||
| text={this.props.content.getUuid() + '-' + title} |
There was a problem hiding this comment.
Does something clever need to be done here (e.g. appending an auto-incrementing number), in case the entry with a given Uuid has two headers with the identical title ?
(or is that guaranteed to never happen?)
P.S. I obviously know nothing about the framework being used here (and it's outside the scope of this particular PR), but I wonder if <AnchorLink href=...> (or <AnchorLink id=...>) would have made it more obvious what this is doing than <AnchorLink text=...> ? (people might assume that a text attribute is what gets displayed to the user?)
There was a problem hiding this comment.
The data structure used for entries guarantees that you never have a duplicated field header on an entry. Using an attribute like hash or fragment would be more semantically correct, but is beyond the scope of this PR.
There was a problem hiding this comment.
The data structure used for entries guarantees that you never have a duplicated field header on an entry.
Are you sure? How does that account for URLs like:
https://resin-io.github.io/scratchpad/#2aa7ca4c-d424-11e7-a9ca-d3e1e018ddf6-treatment-after-resinos-v1-1-2--this-issue-shouldn-t-happen-as-dnsmasq-is-used-as-the-resolver--
There was a problem hiding this comment.
@lurch I don't understand what you mean? Does the link go to a duplicated header?
There was a problem hiding this comment.
Sorry Lucian, my mistake - I thought such a long anchor-link was coming from the ability to use Markdown text inside entries, but looking at the YAML file I see that this actually is the name of one of the fields in that entry.
So I guess what you were saying earlier is that even with user-defined fields, pensieve doesn't allow multiple fields in the same entry to have the same name? (apologies if I'm using the wrong terminology)
There was a problem hiding this comment.
@lurch the entries are esentially defined as JSON objects where the key is the subheader, so as soon as you add a 'header' that already exists, it just overwrites the pre-existing one.
There was a problem hiding this comment.
How does that manifest itself in the pensieve web-interface? Could a user accidentally overwrite an existing field by creating a new field with the same name? (If yes, should I create a new issue for that?)
There was a problem hiding this comment.
I've just tested this in the pensieve web-interface myself, and if you add a user-defined field that has the same name as an existing field, then Pensieve doesn't display any error or warning, and any contents in the existing field get silently discarded :-( I'll add an issue...
EDIT: #266
Fixes #245 change-type: patch
LucianBuzzo
force-pushed
the
245-permalink
branch
from
7ea7d46
to
bad3d2c
Compare
ghost
merged commit 
ghost
removed
the
Fixes #245
change-type: patch