Skip to content

Conversation

@adamraine
Copy link
Contributor

Clicking a score dial, scrolling to the top, then clicking the same score dial doesn't work in the flow report. This is because the URL hash never changes, so it doesn't trigger the flow reports anchors system. This PR uses a normal onclick handler for anchor links in the report.

#11313

@adamraine adamraine requested a review from a team as a code owner October 20, 2021 17:36
@adamraine adamraine requested review from connorjclark and removed request for a team October 20, 2021 17:36
@google-cla google-cla bot added the cla: yes label Oct 20, 2021

export function useHashParam(param: string) {
const [paramValue, setParamValue] = useState(getHashParam(param));
export function useHashParams(...params: string[]) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should have been designed this way initially, so changes to multiple hash params are handled in the same render.


const nodeId = link.hash.substr(1);
link.hash = `#index=${index}&anchor=${nodeId}`;
link.onclick = e => {
Copy link
Member

@paulirish paulirish Oct 20, 2021

Choose a reason for hiding this comment

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

i would expect the new-ish report click handler to end up doing the same thing..

gaugeWrapperEl.addEventListener('click', e => {
if (!gaugeWrapperEl.matches('[href^="#"]')) return;
const selector = gaugeWrapperEl.getAttribute('href');
const reportRoot = gaugeWrapperEl.closest('.lh-vars');
if (!selector || !reportRoot) return;
const destEl = this._dom.find(selector, reportRoot);
e.preventDefault();
destEl.scrollIntoView();
});

but it doesnt?

Copy link
Contributor Author

@adamraine adamraine Oct 20, 2021

Choose a reason for hiding this comment

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

With the flow report, the href of the link is now like #index=0&anchor=seo which isn't a selector anymore.

This would also work if we didn't convert all the link hrefs

@adamraine adamraine merged commit 1d3800c into master Nov 1, 2021
@adamraine adamraine deleted the flow-fix-anchors branch November 1, 2021 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants