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 Analytics.
2. Go to main dashboard.
3. Set viewport between 601px to 664px using dev tool.
4. See issue.

Screenshots

Additional Context

• PHP Version:
• OS: [e.g. iOS]
• Browser: [e.g. chrome, safari]
• Plugin Version: [e.g. 22]
• Device: [e.g. iPhone6]

---

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

• 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)

Test Coverage

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

QA Brief

Changelog entry

[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 <DataBlockGroup> 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