Skip to content

Conversation

@imnasnainaec
Copy link
Collaborator

@imnasnainaec imnasnainaec commented Sep 19, 2023

Resolves #1556 (but not #2616)


This change is Reviewable

@imnasnainaec imnasnainaec self-assigned this Sep 19, 2023
@codecov-commenter
Copy link

codecov-commenter commented Sep 19, 2023

Codecov Report

Attention: 33 lines in your changes are missing coverage. Please review.

Files Coverage Δ
...onents/DataEntry/DataEntryTable/NewEntry/index.tsx 43.75% <ø> (+1.06%) ⬆️
...mponents/Pronunciations/PronunciationsFrontend.tsx 100.00% <100.00%> (ø)
src/components/DataEntry/DataEntryTable/index.tsx 62.53% <0.00%> (+0.74%) ⬆️
src/components/Pronunciations/AudioPlayer.tsx 43.24% <0.00%> (+1.13%) ⬆️
src/components/Pronunciations/AudioRecorder.tsx 27.27% <0.00%> (+7.27%) ⬆️
...als/ReviewEntries/ReviewEntriesComponent/index.tsx 75.00% <66.66%> (ø)
...omponents/DataEntry/DataEntryTable/RecentEntry.tsx 55.00% <0.00%> (ø)
...iesComponent/CellComponents/PronunciationsCell.tsx 71.42% <66.66%> (+71.42%) ⬆️
...omponents/Pronunciations/PronunciationsBackend.tsx 70.00% <70.00%> (ø)
...viewEntriesComponent/Redux/ReviewEntriesActions.ts 62.68% <60.00%> (-0.48%) ⬇️
... and 2 more

... and 1 file with indirect coverage changes

📢 Thoughts on this report? Let us know!.

@imnasnainaec imnasnainaec marked this pull request as ready for review September 19, 2023 22:01
@imnasnainaec imnasnainaec marked this pull request as draft September 19, 2023 22:03
@imnasnainaec

This comment was marked as resolved.

@imnasnainaec imnasnainaec marked this pull request as ready for review September 20, 2023 14:32
Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 12 of 20 files at r1, 3 of 3 files at r2, 6 of 6 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @imnasnainaec)


src/goals/ReviewEntries/ReviewEntriesComponent/CellComponents/PronunciationsCell.tsx line 29 at r3 (raw file):

  return props.audioFunctions ? (
    <Pronunciations

This might be a little too clever... and spacer definitely seems like an inadequate name for what is going on here.
Skipping the record button in the inner Pronunciations by not passing uploadAudio wasn't easy for me to see.
I think we might need to discuss this and other possible design options.

Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 14 of 14 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @imnasnainaec)

Copy link
Collaborator Author

@imnasnainaec imnasnainaec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @jasonleenaylor)


src/goals/ReviewEntries/ReviewEntriesComponent/CellComponents/PronunciationsCell.tsx line 29 at r3 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

This might be a little too clever... and spacer definitely seems like an inadequate name for what is going on here.
Skipping the record button in the inner Pronunciations by not passing uploadAudio wasn't easy for me to see.
I think we might need to discuss this and other possible design options.

Done.

@imnasnainaec imnasnainaec enabled auto-merge (squash) September 25, 2023 21:07
Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @imnasnainaec)

@imnasnainaec imnasnainaec merged commit 4ecb7bd into master Sep 25, 2023
@imnasnainaec imnasnainaec deleted the review-entries-edit-audio branch September 25, 2023 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ReviewEntries] Add/remove audio bumps out of row edit mode and loses changes

4 participants