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

Enable deletion of resolved reference tags #741

Merged
merged 7 commits into from
Dec 8, 2022

Conversation

rabstejnek
Copy link
Collaborator

This is an example of how we can handle deleting resolved tags when conflict resolution is enabled.

When user tags don't exist for a reference, the resolved tags are added by default:

image

From here, a user can add additional tags and also remove resolved tags:

image

If a resolved tag is removed, it is still displayed with a strikethrough. Clicking on it again will reapply it.

@rabstejnek rabstejnek changed the base branch from main to literature-conflict-resolution November 29, 2022 17:24
Copy link
Owner

@shapiromatron shapiromatron left a comment

Choose a reason for hiding this comment

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

A few code changes to simplify the JSX logic

}>
{this.state.showFullTag
? tag.get_full_name()
: tag.data.name}
</span>
))}
{selectedReferenceUserTags
Copy link
Owner

Choose a reason for hiding this comment

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

this is also too complicated; can we generate the list of items as just JS code as a variable above in the render method, not within the JSX component

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This has been simplified after other refactors; since its just a single line filter now I don't think it would make much sense to define it in the render method

border: 0.15rem dotted #ffffff;
color: maroon;
background-color: #ffffff;
border: 0.15rem dotted maroon;
Copy link
Owner

Choose a reason for hiding this comment

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

add some transparency too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I changed the transparency to 0.5, which I think helps make it more readable:
image
Anything less though and the strikethrough doesn't come across very well.

@shapiromatron shapiromatron changed the title Delete resolved tags Add ability to delete refernece tags Nov 30, 2022
@shapiromatron shapiromatron changed the title Add ability to delete refernece tags Enable deletion of resolved refernece tags Nov 30, 2022
@shapiromatron shapiromatron changed the title Enable deletion of resolved refernece tags Enable deletion of resolved reference tags Nov 30, 2022
@rabstejnek
Copy link
Collaborator Author

The conflict resolution page has also been updated to display tags in the same manner as the tagging view:

image

@rabstejnek
Copy link
Collaborator Author

@shapiromatron I did some refactoring outside of your recommendations, which outdates one of your comments; let me know if you like these refactors as a whole and if you're fine with the compromise on your one comment.

@rabstejnek
Copy link
Collaborator Author

Changes look good!

@rabstejnek rabstejnek merged commit 28915f3 into literature-conflict-resolution Dec 8, 2022
@rabstejnek rabstejnek deleted the delete-resolved-tags branch December 8, 2022 19:35
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.

2 participants