docs(PPDSC-2290): Audio player documentation #353

Srinivasan-Ramasamy · 2022-08-30T04:33:15Z

PPDSC-2290

What

Audio player documentation UI - http://ncu-newskit-docs.s3-website-eu-west-1.amazonaws.com/ppdsc-2290-audio-player-documentation/components/audio-player/
Previous audio player page (existing users can still access the docs) - http://ncu-newskit-docs.s3-website-eu-west-1.amazonaws.com/ppdsc-2290-audio-player-documentation/components/audio-player-old/

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:

URL: http://ncu-newskit-docs.s3-website-eu-west-1.amazonaws.com/ppdsc-2290-audio-player-documentation/components/audio-player/

newskit-engineering · 2022-08-30T04:53:27Z

You can preview these changes on:
Documentation Site: http://ncu-newskit-docs.s3-website-eu-west-1.amazonaws.com/ppdsc-2290-audio-player-documentation
Storybook: http://ncu-newskit-docs.s3-website-eu-west-1.amazonaws.com/ppdsc-2290-audio-player-documentation/storybook

Vanals · 2022-09-01T10:11:03Z

Hey Srini it looks good to me! I was just testing the page and noticed the table we use is not responsive. I wonder if is something designers are aware and are testing?
If they agree and is something to fix, could you maybe raise a ticket? Does not look a good experience for certain breakpoints, in some is not readable at all.

table-responsiviness.mp4

Srinivasan-Ramasamy · 2022-09-01T10:41:54Z

Hey Srini it looks good to me! I was just testing the page and noticed the table we use is not responsive. I wonder if is something designers are aware and are testing? If they agree and is something to fix, could you maybe raise a ticket? Does not look a good experience for certain breakpoints, in some is not readable at all.

table-responsiviness.mp4

@Vanals Thank you for taking your time for the review.

Thanks for capturing this scenarios 💯

Current table structure whenever more column with data it showing horizontal scroll bar to look at the hidden column data(Without any blocker to the users). It's very difficult to do the responsive in table with td dom structure. Till 1024 breakpoint, we're hiding the side nav so we can able to the full table. Please suggest?

Here is the few screen shots:

mutebg · 2022-09-02T06:11:35Z

site/pages/components/audio-player.tsx

+          title: 'SelectionList',
+          summary:
+            'The SelectionList has a range of props that can be used to define an appropriate experience for different use cases.',
+          propsRows: [
+            {
+              name: 'children',
+              type: 'React.ReactElement<SelectionListOptionProps>[]',
+              default: '',
+              description: 'An array of SelectionListOptions components.',
+              required: true,
+            },
+          ],
+        },
+        {
+          title: 'SelectionListOption',
+          summary:
+            'The SelectionListOption has a range of props that can be used to define an appropriate experience for different use cases.',
+          propsRows: [
+            {
+              name: 'children',
+              type: 'React.ReactNode',
+              default: '',
+              description: 'Contents of the SelectionListOption',
+              required: true,
+            },
+            {
+              name: 'selected',
+              type: 'boolean',
+              default: 'false',
+              description: 'The selected state of the SelectionListOption',
+            },
+            {
+              name: 'selectedIcon',
+              type: 'React.ReactNode',
+              default: 'IconFilledCheck',
+              description: 'Icon(s) of the SelectionListOption',
+            },
+          ],
+          overridesRows: [


IMO we should not show Selection List and Selection List Option components here. they are internal ones.
We should add only overrides but as part of Speed Control. Maybe we can ask Mike when is back

Sure @mutebg, we will check with Mike and update accordingly. Thanks for your review 👍

Hi @Srinivasan-Ramasamy to Stoyan's point regarding the Selection List and Selection List Option components, I would say we should move the overrides that are currently attributed to both in those sections, under the Playback Speed Control section.

See the updates here in Confluence: [https://nidigitalsolutions.jira.com/wiki/spaces/NPP/pages/3933962254/Audio+Player+Component+-+Web+Documentation#Playback-Speed-Control.1]

It looks like there is a missing inline message under the Playback Speed Control in the API section also to say:

This component should extend the overrides of the Popover and Modal components.

Thanks @messmike for the confirmation 👍

I've updated the changes in same PR url. Please review it once. http://ncu-newskit-docs.s3-website-eu-west-1.amazonaws.com/ppdsc-2290-audio-player-documentation/components/audio-player/

Inline message was kepted inside the Overrides tab, Now I've moved to default visibile.

Thank you @Srinivasan-Ramasamy, this looks great, all looks good to me.

Thanks Mike 👍

messmike · 2022-09-05T08:46:32Z

Hey Srini it looks good to me! I was just testing the page and noticed the table we use is not responsive. I wonder if is something designers are aware and are testing? If they agree and is something to fix, could you maybe raise a ticket? Does not look a good experience for certain breakpoints, in some is not readable at all.

table-responsiviness.mp4

This is a good point @Vanals, we have a ticket in the backlog to look at this: https://nidigitalsolutions.jira.com/browse/PPNKD-531

* docs(PPDSC-2290): Audio player documentation ui.

Srinivasan-Ramasamy added 9 commits August 25, 2022 12:57

docs(PPDSC-2290): audio play initial ui changes

8bf92c0

docs(PPDSC-2290): option section ui changes

d42c60f

docs(PPDSC-2290): ui changes

c88530f

docs(PPDSC-2290): ui changes on behaviour section

1282c60

docs(PPDSC-2290): ui changes on usage section

521dc38

docs(PPDSC-2290): ui changes on accessibility section

f615b6c

docs(PPDSC-2290): accessibility ui changes

56e5a69

docs(PPDSC-2290): updated new layout examples section ui

605852c

docs(PPDSC-2290): updated api section ui

bc345ef

Srinivasan-Ramasamy added docs Improvements or additions to documentation WIP Work in progress labels Aug 30, 2022

Srinivasan-Ramasamy requested review from jps, Xin00163, pdimova, baburay23, mutebg, Vanals and evgenitsn as code owners August 30, 2022 04:33

Srinivasan-Ramasamy and others added 3 commits August 30, 2022 10:11

docs(PPDSC-2290): updated api props and overrides

f0f7a0a

docs(PPDSC-2290): updated api props and overrides

5dd9945

Merge branch 'main' into docs/PPDSC-2290-audio-player-documentation

8307d1a

Srinivasan-Ramasamy added 4 commits August 30, 2022 10:32

docs(PPDSC-2290): ui changes and updated related section

ebde444

docs(PPDSC-2290): ui changes and build fix

9920986

docs(PPDSC-2290): ui changes and build fix

1bd8994

docs(PPDSC-2290): api data fixes

8a47440

Srinivasan-Ramasamy added the ready for design review Please assist in getting this reviewed label Aug 30, 2022

Srinivasan-Ramasamy and others added 2 commits September 1, 2022 08:43

Merge branch 'main' into docs/PPDSC-2290-audio-player-documentation

70b609a

docs(PPDSC-2290): design feedback changes

ff85b2f

Srinivasan-Ramasamy removed the ready for design review Please assist in getting this reviewed label Sep 1, 2022

Srinivasan-Ramasamy added ready for review Please assist in getting this reviewed and removed WIP Work in progress labels Sep 1, 2022

Vanals previously approved these changes Sep 1, 2022

View reviewed changes

Merge branch 'main' into docs/PPDSC-2290-audio-player-documentation

77fcce5

mutebg reviewed Sep 2, 2022

View reviewed changes

Merge branch 'main' into docs/PPDSC-2290-audio-player-documentation

c8989ba

docs(PPDSC-2290): api table ui fixes

1e4f2c4

Srinivasan-Ramasamy dismissed Vanals’s stale review via 1e4f2c4 September 5, 2022 09:37

Srinivasan-Ramasamy requested review from Vanals and mutebg September 5, 2022 10:30

Srinivasan-Ramasamy and others added 3 commits September 5, 2022 20:55

docs(PPDSC-2290): api table ui fixes

9d83ec6

Merge branch 'main' into docs/PPDSC-2290-audio-player-documentation

2b4dce0

Merge branch 'main' into docs/PPDSC-2290-audio-player-documentation

3fb1756

Vanals approved these changes Sep 6, 2022

View reviewed changes

baburay23 approved these changes Sep 6, 2022

View reviewed changes

Srinivasan-Ramasamy merged commit fd0a727 into main Sep 6, 2022

Srinivasan-Ramasamy deleted the docs/PPDSC-2290-audio-player-documentation branch September 6, 2022 09:50

Xin00163 pushed a commit that referenced this pull request Oct 17, 2022

docs(PPDSC-2290): Audio player documentation (#353)

9d8c650

* docs(PPDSC-2290): Audio player documentation ui.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs(PPDSC-2290): Audio player documentation #353

docs(PPDSC-2290): Audio player documentation #353

Srinivasan-Ramasamy commented Aug 30, 2022 •

edited

Loading

newskit-engineering commented Aug 30, 2022

Vanals commented Sep 1, 2022 •

edited

Loading

Srinivasan-Ramasamy commented Sep 1, 2022

mutebg Sep 2, 2022

Srinivasan-Ramasamy Sep 2, 2022

messmike Sep 5, 2022

messmike Sep 5, 2022

Srinivasan-Ramasamy Sep 5, 2022

messmike Sep 5, 2022

Srinivasan-Ramasamy Sep 5, 2022

messmike commented Sep 5, 2022

docs(PPDSC-2290): Audio player documentation #353

docs(PPDSC-2290): Audio player documentation #353

Conversation

Srinivasan-Ramasamy commented Aug 30, 2022 • edited Loading

newskit-engineering commented Aug 30, 2022

Vanals commented Sep 1, 2022 • edited Loading

Srinivasan-Ramasamy commented Sep 1, 2022

mutebg Sep 2, 2022

Choose a reason for hiding this comment

Srinivasan-Ramasamy Sep 2, 2022

Choose a reason for hiding this comment

messmike Sep 5, 2022

Choose a reason for hiding this comment

messmike Sep 5, 2022

Choose a reason for hiding this comment

Srinivasan-Ramasamy Sep 5, 2022

Choose a reason for hiding this comment

messmike Sep 5, 2022

Choose a reason for hiding this comment

Srinivasan-Ramasamy Sep 5, 2022

Choose a reason for hiding this comment

messmike commented Sep 5, 2022

Srinivasan-Ramasamy commented Aug 30, 2022 •

edited

Loading

Vanals commented Sep 1, 2022 •

edited

Loading