Issues with AdSense overview widget responsiveness on 601px to 664px viewports

[mohitwp]

Bug Description

AdSense Module overview widget Highlighted tile content overflowing when viewport size is 601px -664px.

Steps to reproduce

1. Set up site kit with AdSense.
2. Go to main dashboard.
3. Set viewport between 601px to 664px using dev tool.
4. See issue.

Screenshots

---

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

• All "Data Block" groups within Site Kit should automatically and equally shrink the font size of the content within them if any one or more of the containers within a group have content that begins to overflow.

Implementation Brief

• [ ] Create a new component assets/js/components/DataBlockGroup.js
  • It should accept one prop className
  • Create a ref and assign it to the div wrapper that should hold all children. Pass it the className prop
  • Add inline function, adjustFontSize for example, and handle the JS elements query on the current ref (querySelectorAll) for the .googlesitekit-data-block selector
  • Loop through the found elements and using getComputedStyle return the font size property (.fontSize)
  • Get the offsetWidth of the elements parent (you can use .parentElement of the current element, and use a while loop to check if the current element scrollWidth is bigger than parent's offsetWidth, and keep decrement the font size property of the current element for 1px while condition is true. And also for all the elements within the same ref group, so they can match. Each iteration will adjust the font size of any element overflowing the container and match it to other items within the same wrapper.
  • Include useEffect or useLifecycles, so on mount invoke adjustFontSize and add resize listener, to call adjustFontSize as callback, and remove it on unmount. So it can be invoked on screen rotation as well
• [ ] Update assets/js/components/wp-dashboard/WPDashboardWidgets.js
  • Replace the current div wrapper with DataBlockGroup component
• [ ] Update assets/js/modules/adsense/components/module/ModuleOverviewWidget/Overview.js
  • After Row component, add DataBlockGroup and map through headers to render Cell with DataBlock components. You can use the index for the checks and extracting data, which is currently numerically hardcoded for each data block.
• [ ] Update assets/js/modules/analytics-4/components/dashboard/DashboardOverallPageMetricsWidgetGA4.js
  • Wrap the data.map with DataBlockGroup

Test Coverage

• No new tests required.
• Update any failing VRT images

QA Brief

1. Set up Site Kit with AdSense, using a site that has AdSense performance data/history.
2. Go to the Main Site Kit dashboard.
3. Set viewport between 601px to 664px.
4. Verify that values are contained within the container/do not overflow

Changelog entry

• Fix bug that could cause chart legend values to overflow on mobile screen sizes.

[eugene-manuilov]

• The AdSense Module Overview widget should switch to a 2 column layout below the 960px breakpoint.

@jimmymadon, i am not sure that this is the best way to solve this problem. I think it would be much better if we scale the font size for text to fit into the DataBlock container width.

It means that if we have a wider text that the size of the container, we will need to reduce the font size to smaller values that would allow us to display the text without wrapping it. The challenge here is to ensure that the font size is updated for all 4 blocks altogether. This will require some js magic to calculate which font size lets text fit the container. Here are some thoughts how it can be done: https://stackoverflow.com/questions/66046533/scale-font-to-size-when-text-exceeds-container

cc @aaemnnosttv and @tofumatt to chime in if you have a better idea.

[jimmymadon]

@eugene-manuilov Thanks for the feedback and suggestions here.

i am not sure that this is the best way to solve this problem.

https://github.com/google/site-kit-wp/assets/1721319/4210e3f0-aa27-4e7a-82b5-359dae412ac3

You may have already tested this, but just putting a video here to say that it does look ok to me and is a one character fix. Also, the 601 to 960px range is mainly some older/smaller tablets in Portrait mode - so not sure how often we hit this resolution range? Is there any particular reason why this solution isn't good? Do you think there is too much whitespace when we are closer to 960px?

I think it would be much better if we scale the font size for text to fit into the DataBlock container width.

If we do this, we should try to do this fix for all DataBlock containers in the plugin. It does look good to me too - just that it will be some more effort.

[eugene-manuilov]

@jimmymadon

Is there any particular reason why this solution isn't good? Do you think there is too much whitespace when we are closer to 960px?

Yes, if we switch to the 2x2 layout at 960px, we will get a lot of whitespace in data blocks, but the actual graph data will fall off to below the fold area. 960px is wide enough to show four data blocks, we just need to adapt the font size.

I think it would be much better if we scale the font size for text to fit into the DataBlock container width.

If we do this, we should try to do this fix for all DataBlock containers in the plugin. It does look good to me too - just that it will be some more effort.

Yes, that is what I had in mind when I wrote my comment. We need to do it for all data block groups that we have and they should automatically shrink the font size altogether within the group.

[eugene-manuilov]

AC ✔️

[eugene-manuilov]

Update assets/sass/components/global/_googlesitekit-data-block.scss

• When screen size is above the $width-tablet, update the font size to use clamp() and use 32px for min value, 46px (current value) for the max, and 5vw for preferred value so it can scale with the screen. In my quick test 5vw as preferred value had the best result across the screens.
• Bellow the tablet screen size, the current font size should be kept, as layout shifts to the 2 columns
• Example usage: clamp(32px, 5vw, 46px)

@zutigrm, unfortunately, this won't be enough because it will reduce the font site only for one data block. What we need to do here is to ensure that if one data block has overflowing text, then we need to reduce the font size for all four data blocks to make it look consistent. This will require some javascript magic to calculate the proper font size and update all data blocks within the same group.

[zutigrm]

@eugene-manuilov Thanks, I updated IB

[zutigrm]

@eugene-manuilov Taking it back for an update, just remembered that container width is not changing, so we will probably need to track scroll

[eugene-manuilov]

Thanks, @zutigrm, feels like it should do the trick, but maybe let's create a new component that will encapsulate that logic instead of creating one more hook and add it to dashboard components? For example, we can create <DataBlockGroup> and implement that calculation logic within that component for all data blocks that are its children. What do you think?

[zutigrm]

@eugene-manuilov

For example, we can create and implement that calculation logic within that component for all data blocks that are its children. What do you think?

Yeah I was thinking about it at first, but the thing is that we don't have standardised wrapper/loop where we would bring all DataBlock components together. Mostly it is used individually in separate widgets, or used individually within separate Cell wrappers. Some examples WPDashboardSessionDurationGA4.js, WPDashboardUniqueVisitorsGA4.js, ModuleOverviewWidget/Overview.js, and some do have a .map which could be wrapped, but it would be too many places to cover. So I was looking for a way to target all of them, and main dashboards were only choice I could think of. Maybe we could use it in some other entry component, but not sure if there is any better fitting than dashboards, what do you think?

[eugene-manuilov]

but the thing is that we don't have standardised wrapper/loop where we would bring all DataBlock components together. Mostly it is used individually in separate widgets, or used individually within separate Cell wrappers.

I thought about something like this:

export function DataBlockGroup( { children } ) {
    const ref = useRef(0);
    useEffect( () => {
        // find all data blocks that are children of ref.current and calculate 
        // the optimal font size if at least one data block has overlapping text
        ...
    }, [] );

    return <div ref={ ref }>{ children }</div>;
}

export default function WPDashboardWidgets() {
    return (
        <DataBlockGroup ... >
            { analyticsModuleActiveAndConnected && canViewSharedAnalytics && (
                <Fragment>
                    <WPDashboardUniqueVisitorsGA4Widget />
                    <WPDashboardSessionDurationGA4Widget />
                </Fragment>
            ) }
            ...
        </DataBlockGroup>
    );
}

export default function Overview( props ) {
    return (
        <Grid>
            <Row>
                <Cell>
                    <Row>
                        <DataBlockGroup ... >
                            { dataBlocks.map( ( dataBlock, index ) => ( ... ) ) }
                        </DataBlockGroup>
                    </Row>
                </Cell>
                ...
            </Row>
        <Grid>
    );
}

[zutigrm]

@eugene-manuilov Thanks. I updated IB to take that direction of the wrapper. Let me know what you think

[eugene-manuilov]

Thanks. IB ✔️

[mohitwp]

QA Update ⚠️

• Tested on dev environment.
• Verified AdSense overview and other widgets responsiveness.
• Verified that AdSense overview widget issue is resolve now and now widget data is not overflowing.
• @zutigrm On entity dashboard I noticed issue for "Search traffic over the last X Days" widget. I tested it using oi.ie site and details are for Homepage. Should I create a separate ticket for this or you will create follow up PR.


• Note : I've notice issue in "top content over the last 28 days" widget for which I will create a separate ticket.


Pass Case

 https://github.com/user-attachments/assets/bcaa1199-9e5f-4537-8f7f-54b9b9dffa10

[zutigrm]

@mohitwp Thanks for catching this. Yes, this will need separate issue to handle the titles, current issue is handling the values in data block. But part of the current logic for handling the resizing of font size can be probably re-used there, so you can reference this issue in the new one, so it can be considered during definition.

[mohitwp]

As per conversation on slack. Moving this to execution.

[mohitwp]

QA Update ✅

• Tested on dev environment.
• Verified AdSense overview and other widgets responsiveness.
• Verified that AdSense overview widget issue is resolve now and now widget data is not overflowing.
• Verified that "Search traffic over the last X Days" widget issue is resolve now. I tested it using oi.ie site on both main and entity dashboard.

   https://github.com/user-attachments/assets/1da8e71a-6f53-4e16-b664-a4994ee811c1