watson-developer-cloud / discovery-components

IBM Watson Discovery components
https://watson-developer-cloud.github.io/discovery-components/storybook
Apache License 2.0
22 stars 38 forks source link

feat: text active color and PDF highlight/active colors #520

Closed dorianmiller closed 1 year ago

dorianmiller commented 1 year ago

What do these changes do/fix?

This feature:

Examples in the screenshots:

image

How do you test/verify these changes?

As part of the Discovery Tooling:

  1. Load the Discovery tooling
  2. Search for a document
  3. View the document and switch to "advanced view".

Have you documented your changes (if necessary)?

Yes, in code

Are there any breaking changes included in this pull request?

No

CLAassistant commented 1 year ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

noah-eigenfeld commented 1 year ago
Screenshot 2023-07-14 at 9 56 59 AM

Now that I see it in action, I think the styling for selected highlight is a bit cramped. The examples design gave us had generous padding, which means the outline didn't intercept with the actual text like what I'm seeing running this. It also looks like we're using the same opacity for the outline as the unselected highlight, which makes the selected highlight stand out a bit less.

I think we should increase the opacity (or darken the outline color, whichever applies with you implementation) of the selected highlight border, and also ask design if we can add some padding to a selected highlight, such that the border doesn't overlap with the selected highlight's text.

noah-eigenfeld commented 1 year ago
Screenshot 2023-07-14 at 10 06 09 AM

I think you mentioned this in standup yesterday, but why does the PDF view have rounded highlight styling? I thought we wanted to unify the styling between PDF and Text view for our highlights

EDIT: I just saw in your other PR that this is apparently tooling styling overriding components, which we plan to remove before merging https://github.ibm.com/Watson-Discovery/discovery-tooling/pull/9870#issuecomment-59336204

EDIT 2: I commented out the rounding styling on the tooling side, but it looks like there's now a slight offset for where the highlight blocks are being rendered vs where the text is rendered. Idk if that'll be fixed by you removing the tooling styling, but I figured I'd put it on your radar if it wasn't already

Screenshot 2023-07-14 at 2 12 04 PM