Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Inline images feature #462

Merged
merged 116 commits into from
Sep 13, 2024
Merged

Inline images feature #462

merged 116 commits into from
Sep 13, 2024

Conversation

Skalakid
Copy link
Collaborator

@Skalakid Skalakid commented Aug 30, 2024 •
edited
Loading

Details

This PR adds the ✨ inline image preview feature ✨ to the Live Markdown Input on the web.

inline.images.mov

Change log:

  • added inline image previews when inline image tags are present in the input
  • added support for style and resolution customization
  • added loading indicator spinners that appear when the image previews are being loaded
  • added block element wrappers, that wraps the inline image markdown and text that is before it. Thanks to this structure change, the image preview appears as a block below the markdown

Related Issues

GH_LINK

Manual Tests

  1. Insert inline image markdown text into the input. I.e
![demo image](https://picsum.photos/200/300)
  1. Verify cursor positioning and text alignment while writing text around the image preview
  2. Verify image loading spinner animation and behavior
  3. Verify inline image styles, and try changing the styles passed to the component. Especially the max-height and maxWidth of the preview
  4. Verify if text after the preview can be removed on Backspace press
  5. Verify if the Live Markdown Input behaves normally in other situations that come to your mind

Linked PRs

Skalakid added 30 commits June 25, 2024 09:40
@Skalakid
Add refactor changes
f853061
@Skalakid
Fix parserUtils
c4108c1
@Skalakid
Add tree building when creating markdown HTML structure
38693c4
@Skalakid
Refactor function and type names
6a1eaf1
@Skalakid
Fix TS errors
75d6afd
@Skalakid
Update parser structure
db68b78
@Skalakid
Move BrowserUtils into an object
31711ab
@Skalakid
Move utils to separate folder
cab8ca7
@Skalakid
Add block utils
2f829cc
@Skalakid
Move functions from above the web component to utils
b9b2984
@Skalakid
Fix unit tests
3fbc456
@Skalakid
Fix cursor positioning bugs
d74e0e4
@Skalakid
Remove scrollCursorIntoView function
220395b
@Skalakid
Rename variable names
4d95382
@Skalakid
Replace textContent with ref value
907c73b
@Skalakid
Merge branch 'main' into @Skalakid/web-parser-refactor
1017ebc
@Skalakid
Merge branch 'main' into @Skalakid/web-parser-refactor
1f1fefd
@Skalakid
Fix crashes and cursor positioning in E/App
ff41dd7
@Skalakid
Fix copying and pasting text with markdown
d1d8a49
@Skalakid
Fix pasting text starting with newlines
153ead8
@Skalakid
Fix errors when replacing text
ea6aad8
@Skalakid
Fix HTML injestions and pasted text parsing
87953bb
@Skalakid
Fix paste text trimming condiftion
3d30621
@Skalakid
Fix cutting text
471f53a
@Skalakid
Change handlePaste logic to fix newlines in pasted text
92a224c
@Skalakid
Fix cursor positioning when undoing/redoing previously pasted text
c14add4
@Skalakid
Fix text coloring
e9d8ec3
@Skalakid
Add review changes
e24d259
@Skalakid
Move updateTextColor function
092775d
@Skalakid
Fix cursor positioning when changing text and styles at the same time
a0c1dfa
@Skalakid Skalakid requested review from tomekzaw and BartoszGrajdek and removed request for tomekzaw and BartoszGrajdek September 3, 2024 09:39
@Skalakid Skalakid mentioned this pull request Sep 5, 2024
Closed
BartoszGrajdek
BartoszGrajdek reviewed Sep 5, 2024
View reviewed changes
Copy link
Collaborator

@BartoszGrajdek BartoszGrajdek left a comment

Choose a reason for hiding this comment

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

Code looks good, I'm going to test it now 👀

tomekzaw
tomekzaw reviewed Sep 9, 2024
View reviewed changes
WebExample/__tests__/styles.spec.ts Show resolved Hide resolved
parser/index.ts Outdated Show resolved Hide resolved
src/styleUtils.ts Outdated Show resolved Hide resolved
@Skalakid Skalakid requested a review from tomekzaw September 10, 2024 15:08
BartoszGrajdek
BartoszGrajdek previously approved these changes Sep 11, 2024
View reviewed changes
Copy link
Collaborator

@BartoszGrajdek BartoszGrajdek left a comment

Choose a reason for hiding this comment

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

LGTM

@Skalakid Skalakid merged commit e4bb45b into main Sep 13, 2024
5 checks passed
@Skalakid Skalakid deleted the @Skalakid/inline-images-feature branch September 13, 2024 06:12
This was referenced Sep 19, 2024
Open
Open
@nandorojo
Copy link
Contributor

so cool

jjcoffee
jjcoffee reviewed Nov 19, 2024
View reviewed changes
src/web/utils/inputUtils.ts

return text.replaceAll('\r\n', '\n');
// Force letter removal if the input value haven't changed but input type is 'delete'
if (text === target.value && inputType.includes('delete')) {
Copy link

Choose a reason for hiding this comment

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

Coming from Expensify/App#51466, since inputType can be undefined it caused an issue here when trying to call includes on an undefined value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jjcoffee jjcoffee jjcoffee left review comments

@BartoszGrajdek BartoszGrajdek BartoszGrajdek approved these changes

@tomekzaw tomekzaw tomekzaw approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Skalakid @nandorojo @tomekzaw @jjcoffee @BartoszGrajdek