Podcast player #750

bHelland · 2021-01-27T13:26:51Z

Designmanual: ?path=/story/enkle-komponenter--lydavspiller

Fjernet gamle måten å initiere lydavspiller på. Eget script i AudioPlayer som rendrer playeren på nytt med ReactDOM. Dette initieres i ArticleContent
Endret audio-player til å kunne vise bilde, beskrivelse, tekst

Generelt:

Oppgradert TypeScript og eslint til siste versjon og fikset feil som følge av det. TypeScript måtte oppgraderes grunnet avhengig i ekstern pakke
Siden måten audioplayeren rendres på er man avhengig av at article-converter oppgraderes samtidig som front-end oppgraderes

Ser at snyk feiler pga en nøstet avhengighet i "@typescript-eslint/eslint-plugin@4.14.1" og "@typescript-eslint/parser@4.14.1" Regner med at det kommer en fiks på det

…ed components in @ndla/ui Frontpage

ingeborgsteel · 2021-02-02T09:25:21Z

Ikke akkurat krise, men uttale-tabellen ser ikke helt riktig ut i now-deplymenten. Vet ikke om det kan være den layoutwrapperen kanskje?

bHelland · 2021-02-03T09:13:50Z

Ikke akkurat krise, men uttale-tabellen ser ikke helt riktig ut i now-deplymenten. Vet ikke om det kan være den layoutwrapperen kanskje?

Fikset

gunnarvelle · 2021-02-04T07:52:55Z

Snyk-feil kan som regel fikses med å merge inn master i branch.

gunnarvelle · 2021-02-04T08:19:35Z

packages/ndla-ui/src/AudioPlayer/AudioPlayer.tsx

+  };
+  staticRenderId?: string;
+};
+const AudioPlayer = ({


Ser at type er fjerna som props. Nokon spesiell grunn til det?

"type" er ikkje eit attributt på elementet, men kan brukast på underelementer ved fleire format. Vi tar kun imot ei fil så difor brukar vi ikkje

elenaand · 2021-02-04T08:27:27Z

packages/designmanual/stories/pages/SearchResultTypeExample.jsx

@@ -318,6 +318,7 @@ const SearchPageDemo = ({ t }) => {
        dispatchResources({ type: 'RESOURCE_TYPE_SELECTED', ...data });
      }
    }
+    // eslint-disable-next-line react-hooks/exhaustive-deps


hva er det du disabler når du disabler react-hooks/exhaustive-deps? Fra et raskt google søk ser det ut som om det ikke er best practice (facebook/create-react-app#6880 (comment))

disabler følgende warning:
warning React Hook useEffect has a missing dependency: 'resourceItems'. Either include it or remove the dependency array react-hooks/exhaustive-deps

Etter oppgradering av typescript så var det en del slike som dukket opp. Har fikset de fleste, men det er et par i designmanulen som krever noe omskriving, derfor disablet jeg de. Siden de er i designmanualen og ikke i pakkene så tenker jeg at det ikke er så hast med å fikse

gunnarvelle · 2021-02-04T08:56:10Z

Forsøkte å linke inn alle pakker fra denne PR i både article-converter og ndla-frontend og kjøre begge lokalt, men audioplayer vises ikkje. Feilmeldinga som eg får i konsollet er:

Uncaught TypeError: context.formatMessage is not a function
    at t (injectT.js:29)
    at Controls (Controls.js:381)
    at renderWithHooks (react-dom.development.js:14803)

Har du fått dette til å fungere?

bHelland · 2021-02-04T09:58:29Z

Forsøkte å linke inn alle pakker fra denne PR i både article-converter og ndla-frontend og kjøre begge lokalt, men audioplayer vises ikkje. Feilmeldinga som eg får i konsollet er:
Uncaught TypeError: context.formatMessage is not a function
    at t (injectT.js:29)
    at Controls (Controls.js:381)
    at renderWithHooks (react-dom.development.js:14803)
Har du fått dette til å fungere?

@gunnarvelle det har funka ja. Testa vel ikkje etter dei siste commitsa mine. No får eg ei anna feil i article-converter:

[2021-02-04T09:56:02.314Z] ERROR: article-converter/14646 on Bjrnars-MacBook-Pro.local: Invariant failed: You should not use <Link> outside a <Router> Error: Invariant failed: You should not use <Link> outside a <Router> at invariant (/Users/bjornar/Sites/NDLA-frontend-packages/node_modules/tiny-invariant/dist/tiny-invariant.cjs.js:13:11) at Object.invariant [as children] (/Users/bjornar/Sites/NDLA-frontend-packages/node_modules/react-router-dom/modules/Link.js:88:11) at ReactDOMServerRenderer.render (/Users/bjornar/Sites/NDLA-frontend-packages/node_modules/react-dom/cjs/react-dom-server.node.development.js:3635:55) at ReactDOMServerRenderer.read (/Users/bjornar/Sites/NDLA-frontend-packages/node_modules/react-dom/cjs/react-dom-server.node.development.js:3373:29) at renderToStaticMarkup (/Users/bjornar/Sites/NDLA-frontend-packages/node_modules/react-dom/cjs/react-dom-server.node.development.js:4004:27) at renderFunc (/Users/bjornar/Sites/article-converter/src/utils/render.js:23:10) at renderInternal (/Users/bjornar/Sites/article-converter/src/utils/render.js:35:10) at Object.embedToHTML (/Users/bjornar/Sites/article-converter/src/plugins/relatedContentPlugin.js:171:12) at embedToHTML (/Users/bjornar/Sites/article-converter/src/replacer.js:28:33) at callback (/Users/bjornar/Sites/article-converter/src/replacer.js:15:11)

bHelland · 2021-02-04T10:05:14Z

Sjå foreløpig bort fra kommentaren min over. Ser ut til at linkinga mi ikkje fungerer heilt

bHelland · 2021-02-04T11:33:51Z

Linkinga mi er ok. Får feilen i article-converter som nevnt ovanfor på artiklar med relatert innhold. Dette skjer også i master! (f.eks: http://localhost:3000/nn/subject:35/topic:1:190662/resource:1:102239?filters=urn:filter:26a01df2-376e-4090-b0bf-eec2ca3d41dd)

Feilen som @gunnarvelle får klarer eg også å gjenskape på artiklar utan relatert innhold(som då klarer å rendrast av article-converter) i branchen audio-player

EDIT: Mangla linking i article-converter til "react-router" og "react-router-dom". Relatert innhold fungerer difor. Sjå bort ifrå fyrste punkt

bHelland · 2021-02-04T12:43:31Z

Siste commit fikser oversettelse problemet. Bør kanskje lage en egen provider komponent i ndla/ui som kun tar imot locale og håndterer messages selv

gunnarvelle · 2021-02-05T13:54:21Z

Testa siste versjon og no får eg opp lydavspilleren som det skal. Men om eg klikker på Bruk lydfil i avspilleren så klarer eg ikkje lukke dialogen som dukker opp. Ser ikkje kvifor dette skjer.

bHelland · 2021-02-08T12:31:39Z

Testa siste versjon og no får eg opp lydavspilleren som det skal. Men om eg klikker på Bruk lydfil i avspilleren så klarer eg ikkje lukke dialogen som dukker opp. Ser ikkje kvifor dette skjer.

Har du url til artikkelen dette skjer? Eg får ikkje gjenskapt feilen på artiklar som eg testar

gunnarvelle · 2021-02-08T12:40:28Z

I test: https://test.ndla.no/subject:14/topic:1:185588/topic:1:185591/resource:1:75437 Lydavspiller nederst i artikkelen.

bHelland · 2021-02-08T12:49:14Z

Klarer ikkje å gjenskape det lokalt. Alle lydavspelarane i artikkelen virker og får lukka modalen. Testa i Chrome og Safari.

gunnarvelle · 2021-02-08T13:09:21Z

Då er det sikkert berre problemer lokalt hos meg.

gunnarvelle

LGTM.

Kan merge inn master for å fikse snyk evt.

gunnarvelle · 2021-02-08T13:15:22Z

Dette er vel forresten breaking i ndla-ui så vi må passe på det ved publisering.

packages/eslint-config-ndla/base.js

gunnarvelle · 2021-02-11T13:34:01Z

Snyk-feilen er på grunn av en ny avhengihet i es-lint-pakka @typescript-eslint/eslint-plugin. Men det er ingen nyere versjon som løser det så usikker på om dette er et problem.

bHelland · 2021-02-11T13:36:22Z

Snyk-feilen er på grunn av en ny avhengihet i es-lint-pakka @typescript-eslint/eslint-plugin. Men det er ingen nyere versjon som løser det så usikker på om dette er et problem.

Det er ein fiks på veg: gulpjs/glob-parent#36

gunnarvelle · 2021-02-11T13:37:52Z

Snyk-feilen er på grunn av en ny avhengihet i es-lint-pakka @typescript-eslint/eslint-plugin. Men det er ingen nyere versjon som løser det så usikker på om dette er et problem.

Det er ein fiks på veg: gulpjs/glob-parent#36

Kult. Då kan vi jo avvente denne litt til fixen er ute.

gunnarvelle · 2021-03-11T07:53:24Z

packages/ndla-licenses/src/LicenseByline/LicenseDescription.tsx

-LicenseDescription.propTypes = {
-  children: PropTypes.node,
-  licenseRights: PropTypes.arrayOf(PropTypes.string.isRequired).isRequired,
-  messages: PropTypes.shape({


Typescript liker av en eller anna grunn ikkje shape ved validering. Det enkleste var å fjerne propTypes då denne uansett er erstatta av Props.

gunnarvelle · 2021-03-11T07:56:44Z

packages/ndla-ui/src/AudioPlayer/Controls.tsx

+          aria-label="play">
+          <span aria-hidden>
+            {playing ? (
+              <Pause role="img" aria-label="pause" title="play" />


Det som ikkje gir meining er at både role og aria-label er aria props, men TS reagerte kun på role. Så eg måtte legge den til på IconProps for at koden skulle validere.

bHelland added 8 commits January 26, 2021 14:26

Convert AudioPlayer component to TypeScript and styled-components

4444b2f

Podcast player. Rewrite of audioplayer

871019f

Upgrade eslint and fix warnings and errors after upgrade. Remove unus…

f024a33

…ed components in @ndla/ui Frontpage

Styling to controls and added support for text-version in audio-player

0def829

Translations

d434fc7

Example with license

23ef10f

Wide layout

04ca803

Responsive audioplayer

69afdab

bHelland requested review from brdalsnes and gunnarvelle February 1, 2021 11:22

bHelland marked this pull request as ready for review February 1, 2021 11:22

gunnarvelle requested review from ingeborgsteel, haattis, elenaand, hakmet, jnatten, N4G1 and OliverKnowit February 2, 2021 06:03

Fix layout for speech player in designmanual

2e49da2

gunnarvelle reviewed Feb 4, 2021

View reviewed changes

elenaand reviewed Feb 4, 2021

View reviewed changes

Fix i18n problems

0416a3b

bHelland added 3 commits February 4, 2021 14:30

Fix safari spacing

d20e8b0

Fix more safari spacing

1bfe342

LocaleProvider component

aea7759

gunnarvelle approved these changes Feb 8, 2021

View reviewed changes

Merge branch 'master' into audio-player

66cc15a

N4G1 reviewed Feb 10, 2021

View reviewed changes

packages/eslint-config-ndla/base.js Show resolved Hide resolved

gunnarvelle added 3 commits February 11, 2021 09:45

Readding rule but with warning

0a968e6

Merge branch 'master' into audio-player

f1851ec

Removing duplicate export

4f6c5ad

gunnarvelle added 4 commits March 8, 2021 09:38

Temp add glob-parent as dep to fix snyk problem

66ca47b

Merge branch 'master' into audio-player

115e202

Merge branch 'master' into audio-player

def0bfd

Added role to icon. Added optional chaining to ndla-ui

30293be

gunnarvelle reviewed Mar 11, 2021

View reviewed changes

gunnarvelle merged commit 95068f2 into master Mar 11, 2021

gunnarvelle deleted the audio-player branch March 11, 2021 08:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Podcast player #750

Podcast player #750

bHelland commented Jan 27, 2021 •

edited

ingeborgsteel commented Feb 2, 2021

bHelland commented Feb 3, 2021

gunnarvelle commented Feb 4, 2021

gunnarvelle Feb 4, 2021

bHelland Feb 4, 2021

elenaand Feb 4, 2021 •

edited

bHelland Feb 4, 2021

gunnarvelle commented Feb 4, 2021

bHelland commented Feb 4, 2021

bHelland commented Feb 4, 2021

bHelland commented Feb 4, 2021 •

edited

bHelland commented Feb 4, 2021

gunnarvelle commented Feb 5, 2021

bHelland commented Feb 8, 2021

gunnarvelle commented Feb 8, 2021

bHelland commented Feb 8, 2021

gunnarvelle commented Feb 8, 2021

gunnarvelle left a comment •

edited

gunnarvelle commented Feb 8, 2021

gunnarvelle commented Feb 11, 2021

bHelland commented Feb 11, 2021

gunnarvelle commented Feb 11, 2021

gunnarvelle Mar 11, 2021

gunnarvelle Mar 11, 2021

Podcast player #750

Podcast player #750

Conversation

bHelland commented Jan 27, 2021 • edited

ingeborgsteel commented Feb 2, 2021

bHelland commented Feb 3, 2021

gunnarvelle commented Feb 4, 2021

gunnarvelle Feb 4, 2021

Choose a reason for hiding this comment

bHelland Feb 4, 2021

Choose a reason for hiding this comment

elenaand Feb 4, 2021 • edited

Choose a reason for hiding this comment

bHelland Feb 4, 2021

Choose a reason for hiding this comment

gunnarvelle commented Feb 4, 2021

bHelland commented Feb 4, 2021

bHelland commented Feb 4, 2021

bHelland commented Feb 4, 2021 • edited

bHelland commented Feb 4, 2021

gunnarvelle commented Feb 5, 2021

bHelland commented Feb 8, 2021

gunnarvelle commented Feb 8, 2021

bHelland commented Feb 8, 2021

gunnarvelle commented Feb 8, 2021

gunnarvelle left a comment • edited

Choose a reason for hiding this comment

gunnarvelle commented Feb 8, 2021

gunnarvelle commented Feb 11, 2021

bHelland commented Feb 11, 2021

gunnarvelle commented Feb 11, 2021

gunnarvelle Mar 11, 2021

Choose a reason for hiding this comment

gunnarvelle Mar 11, 2021

Choose a reason for hiding this comment

bHelland commented Jan 27, 2021 •

edited

elenaand Feb 4, 2021 •

edited

bHelland commented Feb 4, 2021 •

edited

gunnarvelle left a comment •

edited