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.

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

feat: fetch url meta data on creating row #6130

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

zoli
Copy link
Contributor

@zoli zoli commented Aug 30, 2024

On submitting a new row in board view, if the setting is enabled and the text is a URL fetch url metadata and fill the row with it.

closes #5963

Feature Preview

2024-08-31.00-09-20.mp4

PR Checklist

  • My code adheres to AppFlowy's Conventions
  • I've listed at least one issue that this PR fixes in the description above.
  • I've added a test(s) to validate changes in this PR, or this PR only contains semantic changes.
  • All existing tests are passing.

Copy link

github-actions bot commented Aug 30, 2024

🥷 Ninja i18n – 🛎️ Translations need to be updated

Project /project.inlang

lint rule new reports level link
Missing translation 81 warning contribute (via Fink 🐦)

Copy link

codecov bot commented Aug 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.26%. Comparing base (7ecf213) to head (347d0fc).
Report is 30 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6130      +/-   ##
==========================================
+ Coverage   74.41%   75.26%   +0.84%     
==========================================
  Files         245      232      -13     
  Lines        6566     6828     +262     
  Branches     1057      964      -93     
==========================================
+ Hits         4886     5139     +253     
+ Misses       1628     1626       -2     
- Partials       52       63      +11     
Flag Coverage Δ
appflowy_web_app 75.26% <ø> (+0.84%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zoli
Copy link
Contributor Author

zoli commented Aug 30, 2024

At first, I wanted to also enable this feature in grid view on filling URL field, but filling other fields inside EditableURLCell isn't that straightforward. So I put it aside for maybe in the future.

The next step can be adding the description and image of URL metadata in the row page.

@LucasXu0
Copy link
Collaborator

LucasXu0 commented Sep 9, 2024

After inputting the URL, there's a noticeable delay before the parsed URL is inserted (I understand this is due to fetching metadata). This lag affects the user experience. We should consider a better way to handle this. For example:

  1. insert the raw URL text immediately.
  2. create a new item with the raw URL text.
  3. update the cell content with parsed metadata once it's available.

@zoli What do you think?

Screenshots:
Screenshot 2024-09-09 at 09 34 59
Screenshot 2024-09-09 at 09 35 04
Screenshot 2024-09-09 at 09 35 10

@zoli
Copy link
Contributor Author

zoli commented Sep 9, 2024

@LucasXu0 By creating a new item do you mean sending CreateRow event to the backend? I think creating a pseudo card while fetching the URL is better, Maybe with less opacity so the user knows it's not final yet.

@richardshiue
Copy link
Collaborator

What should the behavior be when there are more than one URL fields?

@zoli
Copy link
Contributor Author

zoli commented Sep 17, 2024

What should the behavior be when there are more than one URL fields?

I can change the settings to choose a specific field, for now, I thought selecting the first URL field type would be good enough.

@zoli
Copy link
Contributor Author

zoli commented Sep 19, 2024

@richardshiue will choosing specific field in settings and pseudo card while fetching the URL be enough for this?

@appflowy appflowy added the tests tasks related to writing tests label Sep 21, 2024
@appflowy
Copy link
Contributor

Hi @zoli , please add bloc tests or integration test.

@zoli
Copy link
Contributor Author

zoli commented Sep 23, 2024

@appflowy, Added both bloc and integration tests.
@richardshiue, Enabled choosing the field (URL type) to fill the URL.
@LucasXu0, Added loading state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests tasks related to writing tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FR] Fetch url meta tags into database fields
4 participants