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: Table navigation using TAB key #627

Merged
merged 9 commits into from
Jan 2, 2024

Conversation

AnsahMohammad
Copy link
Contributor

Implemented the navigation of Table cells using TAB key

This PR solves [FR] Table Cells navigation using TAB key #3982

@CLAassistant
Copy link

CLAassistant commented Dec 21, 2023

CLA assistant check
All committers have signed the CLA.

@Xazin
Copy link
Collaborator

Xazin commented Dec 21, 2023

Would be nice to also have Shift+tab to move the reverse way.

@AnsahMohammad
Copy link
Contributor Author

Would be nice to also have Shift+tab to move the reverse way.

working on that now,
image

Shift+tab isn't working for some reason,

@Xazin
Copy link
Collaborator

Xazin commented Dec 21, 2023

Would be nice to also have Shift+tab to move the reverse way.

working on that now, image

Shift+tab isn't working for some reason,

Maybe shift+tab in lowercase?

@AnsahMohammad
Copy link
Contributor Author

Maybe shift+tab in lowercase?

yeah, the issue was, that hot reload wasn't working so I was confused. It is done now 👍

@AnsahMohammad
Copy link
Contributor Author

Hi @Xazin, could you review my PR when you get a chance? Thank you!

@LucasXu0
Copy link
Collaborator

@AnsahMohammad Can you add the tests to cover the code?

Copy link

codecov bot commented Dec 26, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (739cd59) 78.25% compared to head (66d4077) 78.31%.
Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #627      +/-   ##
==========================================
+ Coverage   78.25%   78.31%   +0.05%     
==========================================
  Files         290      291       +1     
  Lines       12698    12805     +107     
==========================================
+ Hits         9937    10028      +91     
- Misses       2761     2777      +16     

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

@AnsahMohammad
Copy link
Contributor Author

@LucasXu0 I have added tests for Tab, Shift+Tab, Arrow Right and Arrow Left.

@AnsahMohammad
Copy link
Contributor Author

@AnsahMohammad Can you add the tests to cover the code?

ready for review 👍

@LucasXu0
Copy link
Collaborator

LucasXu0 commented Jan 1, 2024

@AnsahMohammad Can you add the tests to cover the code?

ready for review 👍

Nice! I will check it now.

@@ -152,6 +166,42 @@ CommandShortcutEventHandler _downInTableCellHandler = (editorState) {
return KeyEventResult.ignored;
};

CommandShortcutEventHandler _tabInTableCellHandler = (editorState) {
final inTableNodes = _inTableNodes(editorState);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does the _hasSelectionAndTableCell always return true if the result of _inTableNodes is not empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, we check for the following conditions too :

   nodes.length == 1 &&
   selection != null &&
   selection.isCollapsed &&
   nodes.first.parent?.type == TableCellBlockKeys.type;

@LucasXu0 LucasXu0 merged commit 7ec43c9 into AppFlowy-IO:main Jan 2, 2024
10 of 11 checks passed
q200892907 added a commit to q200892907/appflowy-editor that referenced this pull request Jan 4, 2024
* main:
  feat: table navigation using TAB key (AppFlowy-IO#627)
  feat: add markdown link syntax formatting (AppFlowy-IO#618)
  fix:text_decoration_mobile_toolbar_padding (AppFlowy-IO#621)
  fix: active hover on upload image (AppFlowy-IO#597)
  feat: adding an ability to have a link check before embedding (AppFlowy-IO#603)
  fix: node_iterator toList encounter Dangling Node trigger dead loop. (AppFlowy-IO#623)
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.

None yet

4 participants