feat(PPDSE-2360): feat-2360-vertical-volume-control-gap

[baburay23]

PPDSC-2360

What

Please note that this was originally a spike but no investigation was needed in the end, design were happy with the change so i implemented everything here in the ticket

1. Background - There is no space between vertical volume control popover and the mute button
2. What did you do-
   After some attempts we decided to extend the props of the Popover and allow the user to set hidePointer=false on the AudioPlayerVolumeControl.
   By bringing in the Popover Props and Overrides into the AudioPlayerVolumeControl in audioPlayerVolumeControl the user can override the Pointer size and stylepreset, when the pointer is shown the user can hover between the mute button and popover, this is the only way to have the desired behaviour. We decided to give the option to the user to make the pointer transparent that way they can hover inbetween the gap.
   If they create a style preset

const volumeTheme = createTheme({
  name: 'my-custom-volume-theme',
  overrides: {
    stylePresets: {
      customPointerStylePreset: {
        base: {
          backgroundColor: 'transparent',
        },
      },
    },
  },
});

and then add it to the pointer stylePreset override

        <AudioPlayerVolumeControl
          layout="vertical"
          overrides={{
            popover: {
              distance: 'space050',
              pointer: {
                size: 'sizing080',
                stylePreset: 'customPointerStylePreset',
              },
            },
          }}
        />

the pointer will now be transparent and the user can hoover
If they only passed layout=vertical it would look like this

Originally i thought it was a good idea to add the stylePreset for the pointer on our volumeControl defaults but after speaking to other engineers it seems quite hacky and this situation is an edge case. So we thought it might be better to add a storybook scenario instead and the user can set the transparency on the stylePreset themselves

I hope this is ok..if anyone has any opinions otherwise please do let me know.

4. What does the reviewers should expect- overridable space between mute button and popover on volume control vertical

I have done:

• Written unit tests against changes
• Written functional tests against the component and/or NewsKit site
• Updated relevant documentation

I have tested manually:

• The feature's functionality is working as expected on Chrome, Firefox, Safari and Edge
• The screen reader reads and flows through the elements as expected.
• There are no new errors in the browser console coming from this PR.
• When visual test is not added, it renders correctly on different browsers and mobile viewports (Safari, Firefox, small mobile viewport, tablet)
• The Playground feature is working as expected

Before:

After:

Who should review this PR:

How to test:

[newskit-engineering]

You can preview these changes on:
Documentation Site: http://ncu-newskit-docs.s3-website-eu-west-1.amazonaws.com/ppdse-2360-vertical-volumecontrol-gap
Storybook: http://ncu-newskit-docs.s3-website-eu-west-1.amazonaws.com/ppdse-2360-vertical-volumecontrol-gap/storybook

[mutebg]

I think you are on the right path,
However, there is 1 issue. adding this styled block with extra padding makes the whole Block bigger and takes much more space than just a button.
IMO that should not happen.

[Vanals]

Might this be renamed to popoverContent or verticalVolumeSlider? to be more descriptive

[mutebg]

only react component should start with the upper latter

[Vanals]

Just curious, should not be using padding or margin key words here? distance I think we have never used it? is that ok 🤔 ?

[mutebg]

I remember speaking with Mike F about that, its not margin or padding. that's distance between the popover and the button, you still can add padding and margin to the popover/tooltip.

Now I am seeing that probably the right name was "offset" ( that's the name in floating-ui library ) but it's too late since is gonna be a breaking change.

[Vanals]

I see! something maybe to consider for a major? for keeping our props consistent 🤔

[baburay23]

@Vanals will create a ticket for this so that next time we work on major tickets we can fix this!

[baburay23]

https://nidigitalsolutions.jira.com/browse/PPDSC-2428

[mstuartf]

What's the difference between this and just having the Popover component in the return statement as before?

[mutebg]

the idea is to be able to override Popover props ( not only styling ) for example to set hidePointer=false

[mstuartf]

ok, thanks