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: bulk open links shortcut #419

Merged
merged 4 commits into from
Sep 11, 2023
Merged

Conversation

Tony-MK
Copy link
Contributor

@Tony-MK Tony-MK commented Aug 28, 2023

Created a keyboard shortcut to bulk open selected links in a document.
Reference issue: #2323

@CLAassistant
Copy link

CLAassistant commented Aug 28, 2023

CLA assistant check
All committers have signed the CLA.

@Tony-MK Tony-MK changed the title Bulk open links shortcut feat: bulk open links shortcut Aug 28, 2023
@LucasXu0
Copy link
Collaborator

@Tony-MK please check the tests. some of them failed.

@Tony-MK
Copy link
Contributor Author

Tony-MK commented Aug 28, 2023

Thanks @LucasXu0, I am currently trying to fix them, then it will be ready for review.

@Tony-MK Tony-MK marked this pull request as ready for review August 28, 2023 16:01
@codecov
Copy link

codecov bot commented Aug 29, 2023

Codecov Report

Patch coverage: 11.76% and project coverage change: -0.77% ⚠️

Comparison is base (a9af2bb) 80.61% compared to head (eb58b7d) 79.85%.
Report is 20 commits behind head on main.

❗ Current head eb58b7d differs from pull request most recent head 74dba32. Consider uploading reports for the commit 74dba32 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #419      +/-   ##
==========================================
- Coverage   80.61%   79.85%   -0.77%     
==========================================
  Files         255      275      +20     
  Lines       10441    11474    +1033     
==========================================
+ Hits         8417     9162     +745     
- Misses       2024     2312     +288     
Files Changed Coverage Δ
...ts/command_shortcut_events/open_links_command.dart 6.25% <6.25%> (ø)
...tor/block_component/standard_block_components.dart 100.00% <100.00%> (ø)

... and 50 files with indirect coverage changes

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

@Tony-MK
Copy link
Contributor Author

Tony-MK commented Sep 1, 2023

Sorry for the wait, @LucasXu0. I don't think I have any better ideas. Your suggestions are good.
However, I do have a question regrading the best time to call the code snippet below.

for (final link in links) { safeLaunchUrl(link); }

What difference does it make if the function loops through openedLinks and called after the initial loop has finished? Despite the openedLinks set having more items than a node's links set, the snippet will only be called once so I believe it should be faster and also probably be prettier to read. If I am wrong, can you please explain why and what is the difference?

@LucasXu0
Copy link
Collaborator

LucasXu0 commented Sep 1, 2023

@Tony-MK You're right. It would be better to call the "open link" function after the loop.

  // A set to store the links which should be opened
  final links = nodes
      .map((e) => e.delta)
      .whereNotNull()
      .expand(
        (e) => e.map<String?>(
          (op) => op.attributes?[AppFlowyRichTextKeys.href],
        ),
      )
      .whereNotNull()
      .toSet();

  for (final link in links) {
    safeLaunchUrl(link);
  }

@Tony-MK
Copy link
Contributor Author

Tony-MK commented Sep 1, 2023

Thanks @LucasXu0, I will commit the changes ASAP.

@LucasXu0 LucasXu0 merged commit 2924c16 into AppFlowy-IO:main Sep 11, 2023
q200892907 added a commit to q200892907/appflowy-editor that referenced this pull request Sep 13, 2023
* main:
  chore: refactor attribute comparison in Delta class diff loop (AppFlowy-IO#456)
  fix: duration cannot be zero in animate (AppFlowy-IO#452)
  chore: update Chinese l10n (AppFlowy-IO#445)
  fix: sometimes failed to paste content from google translation (AppFlowy-IO#451)
  feat: refactor color conversion method to handle RGB and hex formatsRefactor method to handle RGB and hex formats, improving color conversion (AppFlowy-IO#450)
  fix: unable to paste html contains section (AppFlowy-IO#448)
  fix: remove unused check in non_delta_input_service (AppFlowy-IO#447)
  feat: optimize the performance (AppFlowy-IO#442)
  refactor: migrate tests (AppFlowy-IO#438)
  feat: bulk open links shortcut (AppFlowy-IO#419)
  fix: request focus in find replace menu (AppFlowy-IO#440)
  feat: implement delta diff and provide external values (AppFlowy-IO#444)
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.

3 participants