Skip to content

Fix console error in forked-to - #4327

Merged
yakov116 merged 3 commits into
mainfrom
forked_to
May 24, 2021
Merged

yakov116 merged 3 commits into
mainfrom
forked_to

Conversation

@yakov116

@yakov116 yakov116 commented May 7, 2021

Copy link
Copy Markdown
Member

Fixes #4315

How to test

Have 2 forks and do history.back()

Screenshot

None

@yakov116 yakov116 added the bug label May 7, 2021
@fregante

fregante commented May 8, 2021

Copy link
Copy Markdown
Member

Forked-details already contains a forked-button. I added that class on both for this specific reason, why isn’t it matched?

@yakov116

yakov116 commented May 9, 2021

Copy link
Copy Markdown
Member Author

Good question I dont know 😕. Maybe it fires too early for it to be caught?

@fregante

fregante commented May 9, 2021

Copy link
Copy Markdown
Member

Thing is, if that isn't matched, than this shouldn't match either.

@fregante fregante closed this May 9, 2021
@yakov116

yakov116 commented May 9, 2021

Copy link
Copy Markdown
Member Author

But it does I dont get an error after this change!

@fregante

fregante commented May 9, 2021

Copy link
Copy Markdown
Member

Screen Shot

This PR is adding a rgh-forked-details class immediately before the existing rgh-forked-button element; It's just duplicating existing functionality. I'd rather see a PR that figures out why the existing check does not work.

@yakov116
yakov116 deleted the forked_to branch May 9, 2021 11:48
@yakov116
yakov116 restored the forked_to branch May 21, 2021 14:36
@yakov116 yakov116 reopened this May 21, 2021
@yakov116

yakov116 commented May 21, 2021 •

Copy link
Copy Markdown
Member Author

Figured it out. Dropdown does not have the leading slash.

@yakov116
yakov116 requested a review from fregante May 24, 2021 02:40

@fregante fregante left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@fregante

Copy link
Copy Markdown
Member

Actually, I’m confused. They’re created with the same createLink function, how can they be different?

@fregante

Copy link
Copy Markdown
Member

Dropdown does not have the leading slash

Rather than dropdown it's that the URLs are /user/repo on the root and /user/repo/blob/file on other pages. So sometimes it matched, sometimes it didn't. It sounds like it was not related to the dropdown at all

@yakov116

Copy link
Copy Markdown
Member Author

Rather than dropdown it's that the URLs are /user/repo on the root and /user/repo/blob/file on other pages. So sometimes it matched, sometimes it didn't. It sounds like it was not related to the dropdown at all

Correct

@yakov116
yakov116 merged commit 4d602fb into main May 24, 2021
@yakov116
yakov116 deleted the forked_to branch May 24, 2021 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

forked-to: Cannot set property 'href' of undefined

2 participants