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

feat(page): support text highlight #3961

Merged
merged 7 commits into from
Aug 11, 2023
Merged

feat(page): support text highlight #3961

merged 7 commits into from
Aug 11, 2023

Conversation

fourdim
Copy link
Contributor

@fourdim fourdim commented Aug 10, 2023 •
edited

This closes #2315.
Depend on toeverything/design#45.
image

@vercel
Copy link

vercel bot commented Aug 10, 2023 •
edited

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
blocksuite ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 11, 2023 6:53am
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
blocksuite-docs ⬜️ Ignored (Inspect) Visit Preview Aug 11, 2023 6:53am

@doodlewind doodlewind added the notable Major improvement worth emphasizing label Aug 11, 2023
doodlewind
doodlewind reviewed Aug 11, 2023
View reviewed changes
Copy link
Member

@doodlewind doodlewind left a comment

Choose a reason for hiding this comment

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

Not sure how to add highlight from UI, these items seems to be invalid for now:

image

@fourdim
Copy link
Contributor Author

fourdim commented Aug 11, 2023

Not sure how to add highlight from UI, these items seems to be invalid for now:

image

It depends on toeverything/design#45.

Saul-Mirone
Saul-Mirone requested changes Aug 11, 2023
View reviewed changes
Copy link
Collaborator

@Saul-Mirone Saul-Mirone left a comment

Choose a reason for hiding this comment

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

Please add e2e test for highlight feature.

@fourdim
Copy link
Contributor Author

fourdim commented Aug 11, 2023

Is the name bghighlight in packages/virgo/src/utils/base-attributes.ts ok for you? Please notice once this is merged, changing this name may be a breaking change.

@Saul-Mirone
Copy link
Collaborator

Is the name bghighlight in packages/virgo/src/utils/base-attributes.ts ok for you? Please notice once this is merged, changing this name may be a breaking change.

@Flrande

@Flrande
Copy link
Member

Flrande commented Aug 11, 2023

Is the name bghighlight in packages/virgo/src/utils/base-attributes.ts ok for you? Please notice once this is merged, changing this name may be a breaking change.

I think it will be more appropriate to add it in affineTextAttributes and use background instead of bghighlight

@Flrande Flrande changed the title feat(virgo): support text highlight feat(page): support text highlight Aug 11, 2023
Flrande
Flrande approved these changes Aug 11, 2023
View reviewed changes
packages/virgo/src/utils/base-attributes.ts Outdated Show resolved Hide resolved
@Saul-Mirone Saul-Mirone merged commit 0ff9ae3 into toeverything:master Aug 11, 2023
18 checks passed
@doodlewind doodlewind mentioned this pull request Aug 16, 2023
Closed
@fourdim fourdim deleted the feat-highlight branch August 29, 2023 03:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Saul-Mirone Saul-Mirone Saul-Mirone approved these changes

@Flrande Flrande Flrande approved these changes

@doodlewind doodlewind Awaiting requested review from doodlewind

Assignees

@fourdim fourdim

Labels
notable Major improvement worth emphasizing
Projects
AFFiNE Project
Archived in project
BlockSuite Project
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support text highlight
4 participants
@fourdim @Saul-Mirone @Flrande @doodlewind