oracc / oracc-search-front-end

1 stars 1 forks source link

history,state.data doesn't seem to exist, so removed it. #60

Closed tim-band closed 7 months ago

tim-band commented 8 months ago

So, James' breadcrumbs code removed the code that sets breadcrumb.data to history.state.data. I have done some investigation on history and history.state and it seems history.state.data does not exist. Perhaps it does in some versions of some browsers, but it seems to me that history.state is a browser-defined object.

So I just stripped all that stuff out.

Do you like it?

tim-band commented 8 months ago

Code looks good to me. Are we confident that history.state.data isn't referring to some Angular concept? In my manual testing the history logic didn't seem to change how the app functioned, so I agree that I think we can remove it.

As long as removing it doesn't change the apps functionality, happy to go ahead with this.

Ah, you are right, it is an Angular thing. I don't know what it's supposed to do, and it defaults to something stored in a cookie which is why we find it still works if we don't populate it (I think). So since the /search fix we don't use it. If we're happy with that then this is OK.

tim-band commented 8 months ago

I think I am coming close to figuring this out. This data is set in the line

this.router.navigate([this.router.url, 'source'], {
  state: { data: this.unsanatizedMetadataPanel }
});

in two places in details-texts.component.ts

(Just noticed the spelling mistake in unsanitized, anyway) so it seems we are putting HTML into this data member. I don't really know why this is all happening, I assume it's an optimization, to avoid pulling this HTML down from build-oracc multiple times. We probably don't need the optimization.

jhughes982 commented 8 months ago

From my perspective, if we really don't need this optimisation anymore lets remove it. I didn't spot any difference in app usability. Happy for you to go ahead as you see best.

tim-band commented 7 months ago

This PR has been overtaken by navigation-fixes (PR #62)