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

As a reader, I want to read notes in context as I'm reading an article so that I can refer to additional information without losing my place. #14

Closed
12 tasks done
thatbudakguy opened this issue Mar 16, 2020 · 44 comments
Assignees
Milestone

Comments

@thatbudakguy
Copy link
Collaborator

thatbudakguy commented Mar 16, 2020

testing notes

  • notes should be full width up to 414px
  • when notes are full width, they should have a slide up/down transition
  • when notes are not full width, they should be position so the arrow is under the reference and they should fade in/out
  • notes should disappear automatically when you scroll
  • if you click on a reference near the bottom of the page and the note is not fully visible, the page should scroll up so you can see the note; the note position should adjust to match the scroll
  • on desktop, if the reference is close to the left side of the page the triangle shape should be flipped (to ensure there is space for arrow & reference to align) [might have to adjust browser window size to get a reference so it just wraps)
  • clicking the x icon on an open note should close the icon and not jump you to the reference
  • endnotes should be visible when you scroll all the way down; backlinks should take you to the reference in the text
  • if you load the page with a note id in the url, the note should display

dev notes

figma link
this corresponds to the ability to add footnotes, contextual notes, etc - however we implement that. it could involve a shortcode, as suggested in #4.

revisions:

  • full width up to 414px
  • change reference link underline style – 5px wide, with a 60% transparency with gradient
  • always show reference underline, not just on hover
  • fix number alignment under point of arrow
  • fix outline display issue on safari
  • fix link color
  • fix note position on mobile
  • override close button so document doesn't scroll to reference anchor
  • slide up/down transition on mobile
  • fade in/out transition on tablet/desktop
  • swap arrow icon for x icon
  • set class and styles for notes when displayed as end notes
@thatbudakguy thatbudakguy modified the milestone: v0.1 Mar 16, 2020
@rlskoeser
Copy link
Contributor

Currently implemented with standard markdown footnotes. There are two examples in the current Data Beyond Vision article draft.

@rlskoeser
Copy link
Contributor

I noticed that the text version of the articles includes some junk at the end of the footnotes — I think this is the return to text link generated by the markdown tool. Maybe we can just configure markdown not to generate one, since I don't think we need it based on Gissoo's design. I spent a little time trying to remove it with a "replace" call in the template but it didn't work and I decided it made sense to hold off until we implement this.

FWIW, I think this issue title is too vague

@thatbudakguy
Copy link
Collaborator Author

agree issue title is too vague. wanted something that would imply contextual notes, footnotes, etc. without specifying the implementation, but it didn't come out clearly.

@thatbudakguy thatbudakguy changed the title As a reader, I want to view additional information about a block of text. As a reader, I want to view contextual notes as I'm reading an article. Mar 23, 2020
@thatbudakguy thatbudakguy added this to the v1.0 milestone Mar 27, 2020
@thatbudakguy thatbudakguy modified the milestones: v1.0, v0.1 Apr 9, 2020
@rlskoeser rlskoeser self-assigned this Apr 15, 2020
@rlskoeser
Copy link
Contributor

Notes from my discussion / working session with @gissoo earlier this week when we looked at my work in progress implementation.

In general:

  • It's better if the note is close to where you are already reading
  • It's preferred to have the note near the reference

Contextual notes should be full-width on mobile; on tablet they should be 320px wide. Desktop will increase a bit because font-size will be bigger (size/specifics TBD). Tablet and desktop will work similarly in terms of placement and behavior.

Ideally, on tablet/desktop the contextual note should be placed so the arrow is directly under the reference. This will require javascript for placement, and we'll also need to mirror-image the arrow shape if the reference is close to the left margin. (Gissoo would prefer this if it's not too difficult; my suspicion is that if we're already calculating placement relative to the reference it isn't that much harder.)

When I asked about transitions: on mobile, it would make sense to pull up from the bottom, but it doesn't make sense on the other screens. Gissoo thought maybe a fade could work, but not sure how much it adds.

In terms of behavior: we discussed closing the contextual note after the ref is no longer visible, but then Gissoo suggested it could just go away on scroll - which seems logical (if you start scrolling you're probably done looking at the note), and also easier to implement.

rlskoeser added a commit that referenced this issue Apr 16, 2020
Initial contextual note implementation (CSS + JS positions, no transitions) #14
@gissoo
Copy link
Contributor

gissoo commented Apr 16, 2020

@thatbudakguy @rlskoeser is it intentional that the figma link here is going to the "information architecture" page on figma?

@rlskoeser
Copy link
Contributor

rlskoeser commented Apr 16, 2020

@gissoo @gwijthoff initial implementation of contextual notes is ready for review. The visuals aren't fully to spec since those were still being worked on, but I wanted to share what I've got so far so that we can all try them out and see what needs to be refined.

The functionality is implemented basically as @gissoo and I discussed, documented in a comment above (@gissoo including flipping the triangle for notes close to the left margin!), but I have not yet implemented any transitions.

Gissoo: Here is the link to look at those comments

@rlskoeser
Copy link
Contributor

@thatbudakguy @rlskoeser is it intentional that the figma link here is going to the "information architecture" page on figma?

@gissoo it was going to the contextual notes in the component before, but maybe you removed that when you added the new version?

@rlskoeser
Copy link
Contributor

It occurred to me I should document how I expect this to behave to guide review and feedback:

  • on mobile, contextual notes should be displayed at the bottom and not positioned relative to the footnote reference (we still need to refine our definition of mobile screen size; larger phones may currently get tablet behavior)
  • on tablet/desktop only:
    • contextual notes should be displayed with the arrow of the triangle directly under the
      reference
    • if the reference is close to the left side of the page, the contextual note shape should be flipped and positioned correctly under the reference
    • if the contextual note is not fully visible (e.g. because the reference was near the bottom of the screen when you clicked on it), the page should scroll up and the note should be repositioned so it is fully visible
  • on all screens, contextual notes should disappear when you scroll
  • on all screens, clicking on the down arrow in the contextual note will close the note and take you back to the reference
  • you should be able to load to a page with a footnote reference in the url and have the contextual note displayed correctly
  • all footnotes should display normally at the end, with correct numbering, and you should be able to link back to the reference in the text

@thatbudakguy thatbudakguy assigned gissoo and gwijthoff and unassigned rlskoeser Apr 22, 2020
@gissoo
Copy link
Contributor

gissoo commented Apr 22, 2020

  • @rlskoeser thank you so much for working on this!! I have updated the design belonging to the issue (Revise Article Contextual Notes #51) that is blocking this one.
    Everything you have mentioned makes a lot of sense to me, I appreciate it all!!

  • just to make sure, as a potential reminder, the contextual note container on desktop will still be 320px, the height is what will increase due to the font-size, not the width.
    (mentioning for clarity because you mentioned in your third comment above "Contextual notes should be full-width on mobile; on tablet they should be 320px wide. Desktop will increase a bit because font-size will be bigger (size/specifics TBD)" )

  • looking at how it transitions when selected, I think a fade would make sense! It looks somewhat intense and rough the way it opens.

  • I know you mentioned not all aspects are implemented, when I tried to shrink my browser window on desktop it doesn't look right - should I be testing for this right or just ignore for now?

@rlskoeser
Copy link
Contributor

@gissoo what are you seeing/not seeing? On mobile the contextual notes should be positioned at the bottom per your design, but if you have a footnote open when you resize the browser it might not adjust (or it might even require a reload...).

Did you see my comment above with a list of what I expect to be working?

Thanks for clarifying the desktop size - my notes were provisional, and I'll implement based on your revisions when I return to this.

I agree the show/hide is pretty rough right now. Do you still want fade for tablet/desktop and slide (?) from the bottom for mobile?

Other questions that have occurred to me:

  • Should there be a minimum height? (for small notes I'm not sure the shape is correct, but maybe there is another way to adjust)
  • If the note is short, where should the text be placed?
  • Should there be a maximum height?
  • If there is a maximum, how do we handle content that is longer, if that occurs? @thatbudakguy suggested the contents within the note could scroll

@gissoo
Copy link
Contributor

gissoo commented Apr 23, 2020

@gissoo what are you seeing/not seeing? On mobile the contextual notes should be positioned at the bottom per your design, but if you have a footnote open when you resize the browser it might not adjust (or it might even require a reload...).

Trying this now – will make a separate comment with test results

Did you see my comment above with a list of what I expect to be working?

@rlskoeser Thank you! Sorry I didn't catch the vert first line you had written there! Got it!

Thanks for clarifying the desktop size - my notes were provisional, and I'll implement based on your revisions when I return to this.

I agree the show/hide is pretty rough right now. Do you still want fade for tablet/desktop and slide (?) from the bottom for mobile?

Yes, that sounds right!

Other questions that have occurred to me:

* Should there be a minimum height? (for small notes I'm not sure the shape is correct, but maybe there is another way to adjust)

I think so, I'm noticing the text is too much at the bottom when the note is short. I think the shape is correct

* If the note is short, where should the text be placed?

I think there needs to be some more spacing from the bottom of the container so the text is not way at the bottom, I'm proposing on Figma here

* Should there be a maximum height?

I can think of it two ways: 1. the max height will always be from the bottom of the superscript to the bottom of the screen on the device 2. there won't be any max height, meaning the user will scroll down to see the rest of the note, but that is problematic because the scroll makes the container disappear! Unless we can do scroll within a container, which would be amazing!

* If there is a maximum, how do we handle content that is longer, if that occurs? @thatbudakguy suggested the contents within the note could scroll

Woah!! Amazing!! :D This would be awesome!

@gissoo
Copy link
Contributor

gissoo commented Apr 23, 2020

It occurred to me I should document how I expect this to behave to guide review and feedback:

Thank you, @rlskoeser, below are the results I'm getting. Note: I'm still unable to test with firefox and chrome on my phone – but testing on my phone with Safari and on desktop with Firefox and Chrome and Safari (different window sizes)

* on mobile, contextual notes should be displayed at the bottom and not positioned relative to the footnote reference (we still need to refine our definition of mobile screen size; larger phones may currently get tablet behavior)

This is working fine on the narrowest window on desktop on all browsers, however, it's not working correctly on my phone on Safari. Please see the attached screen recording. (Update: github is not supporting the attachment format, I'll see once you respond where I should place it)

* on tablet/desktop only:
  
  * contextual notes should be displayed with the arrow of the triangle directly under the
    reference

This is working fine.

  * if the reference is close to the left side of the page, the contextual note shape should be flipped and positioned correctly under the reference

this is working perfectly! Thank you!!

  * if the contextual note is not fully visible (e.g. because the reference was near the bottom of the screen when you clicked on it), the page should scroll up and the note should be repositioned so it is fully visible

This is also working fine.

* on all screens, contextual notes should disappear when you scroll

This is working fine.

* on all screens, clicking on the down arrow in the contextual note will close the note and take you back to the reference

This is working fine too, just thinking if we could change the direction of the arrow to be upwards when the container is open? – It's not intuitive now. Is it possible? Should I revise design for this?

* you should be able to load to a page with a footnote reference in the url and have the contextual note displayed correctly

Yes!

* all footnotes should display normally at the end, with correct numbering, and you should be able to link back to the reference in the 

text

I wonder to help it be more visible, it would be nice to "highlight" the text (like an actual highlighter) when it's scrolled up to it, it's more intuitive than just scrolling to it.

A couple of other issues I'm noticing:

  1. With short notes, the text appears way at the bottom of the container.
    1

  2. The angle of the container in note 5 is not correct, I think because the note is so long the container is being resized by being stretched rather than only its height being increased.
    2

  3. When resizing my browser window even after refreshing as you advised the container appears too wide.
    3

  4. The icon with the image is so cool! It would be nice to identify use cases and make some guidelines, I think it can visually improve and not be on the same line as the text with the size that it has and/or be differently aligned with the text.
    4

  5. On mobile and desktop with Safari, the underline style is not correct, it has three sides.

5

  1. Could we try a solid line for underline? I am not sure about the gradient as I keep looking at it more, also wondering if the line is better if it's thicker.

@gissoo
Copy link
Contributor

gissoo commented Apr 23, 2020

@thatbudakguy @rlskoeser is it intentional that the figma link here is going to the "information architecture" page on figma?

@gissoo it was going to the contextual notes in the component before, but maybe you removed that when you added the new version?

You just taught me something new!!! But no, that wasn't why, still should work if the "Frame" is there and the object inside the frame is replaced, don't know what might have happened.

@gwijthoff
Copy link
Contributor

Not sure if these thoughts been aired already, but I'll echo them just in case:

On mobile, the gradient underline wraps around the word in a rectangle. (See Gissoo's screenshot above.) But on a laptop, it's just a straight underline.

When a user clicks the down arrow to close out the note, the page auto-scrolls down so that the top of the window is aligned with the annotated line, which is distracting.

Also: on mobile, when I open a contextual note and then scroll down on the page, the note closes out. But if I try to click that note again, it won't open.

@rlskoeser
Copy link
Contributor

@gissoo I think some of these display issues are impacted that we haven't decided what the largest size screen is we want for mobile - I think you and @thatbudakguy had some discussion about it but I don't know if we settled on a width. Current implementation is using one of our breakpoints, so I think it switches out of mobile too soon.

Weird about the box instead underline! Thanks for catching.

@gissoo I can try a solid line, I just thought the gradient would be better because it's an arbitrary width that won't necessarily match up with any words. Could you suggest a width you'd like to see?

@gissoo I want to include a color palette (which might be easiest as an image, but could also work at html/css) in one of our DBV essay footnotes; could you use that to come up with some guidelines? I agree it would be better if the image was the width of the note; this was just a convenient image to try to make sure it worked and start us thinking about it.

@gissoo thoughts on the down arrow auto-scrolling? @thatbudakguy commented on this too. It seems to behave even more poorly on mobile. My thought is to override default behavior in javascript so it doesn't scroll any but just closes the note.

@gwijthoff which browser for that mobile behavior where you can't click on the note again? (I can't duplicate on android chrome or FF).

@gissoo
Copy link
Contributor

gissoo commented Apr 27, 2020

@gissoo I think some of these display issues are impacted that we haven't decided what the largest size screen is we want for mobile - I think you and @thatbudakguy had some discussion about it but I don't know if we settled on a width. Current implementation is using one of our breakpoints, so I think it switches out of mobile too soon.

  • @rlskoeser I found the comment about this!! (Revise Article Contextual Notes #51 (comment))
  • So, we agreed on 414px being the largest size, however I was suggesting if we could test it together to see how much it works out visually for the text within the container and if it would need to be a little bit styled or if it's even possible for it to be styled for a wider mobile screen (issues with its wrapping/spacing) – Also, here – Nick agreed that we make the container 320px for tablet and desktop.

Weird about the box instead underline! Thanks for catching.

@gissoo I can try a solid line, I just thought the gradient would be better because it's an arbitrary width that won't necessarily match up with any words. Could you suggest a width you'd like to see?

Ah, I see! Could we try this?: How about it would be 5px wide and 30% with a transparency of 40%, with no gradient? I tested it here, I think it helps to look at it here since these numbers may not be completely reliable.

Or what about this? – 5px wide, with a 60% transparency with gradient? – This looks much better to me! I think the gradient makes a lot of sense and looks much better when the line is thicker.

@gissoo I want to include a color palette (which might be easiest as an image, but could also work at html/css) in one of our DBV essay footnotes; could you use that to come up with some guidelines? I agree it would be better if the image was the width of the note; this was just a convenient image to try to make sure it worked and start us thinking about it.

Ah sorry I totally forgot about this specific use case! Yes, I'll add some guidelines!

@gissoo thoughts on the down arrow auto-scrolling? @thatbudakguy commented on this too. It seems to behave even more poorly on mobile. My thought is to override default behavior in javascript so it doesn't scroll any but just closes the note.

I agree, let's do that. I think it's better if instead of an arrow we just have an "x". What do you think? Should I revise it?

@thatbudakguy
Copy link
Collaborator Author

@rlskoeser correct, 414px is a new value. there's currently not a good way to share between the scss and javascript...i could see doing it using hugo's pipes where you can inject config variables into files, but when I tried that for scss it was unable to handle globbing/multiple scss files. I'm fine with just defining it in two separate places.

@rlskoeser rlskoeser assigned rlskoeser and unassigned gwijthoff and gissoo Jun 1, 2020
@rlskoeser rlskoeser changed the title As a reader, I want to view contextual notes as I'm reading an article. As a reader, I want to read notes in context as I'm reading an article so that the notes are available when I want them without losing my place.. Jun 11, 2020
@rlskoeser rlskoeser changed the title As a reader, I want to read notes in context as I'm reading an article so that the notes are available when I want them without losing my place.. As a reader, I want to read notes in context as I'm reading an article so that I can refer to additional information without losing my place. Jun 11, 2020
@rlskoeser
Copy link
Contributor

Upgrading from 5 points to 8 based on the increased complexity of transitions and other behaviors.

@gissoo
Copy link
Contributor

gissoo commented Jun 11, 2020

@rlskoeser thank you so much for working on this!! The underline and the transition are amazing!! You're the best =) There are three issues I'm seeing:

  1. the gradient line that's under the text is so close to the line below it, can we bring it closer to the line it's supposed to appear under? (y axis)
  2. in the data beyond vision essay, the 5th contextual note container's angle is still very distorted, it's not supposed to be stretched, is there a way to solve it so that the angle remains the same and just the length of the container changes to match the design? Same thing is happening with very short contextual ones, they also have a weird angle at the top.
  3. The container does not work properly on any of the iOS browsers. See images below, 1. when opening the dbv essay page on iOS a contextual note is open by default, appearing at the bottom. Contextual note 5 is cut off from the top, I think it would make sense that it gets cut off from the bottom when it's too long of a note, I think the user should always see the piece of text with the superscript.
    Image from iOS (3) copy
    Image from iOS (3)

@rlskoeser
Copy link
Contributor

@gissoo thanks for testing!

  1. I couldn't figure out from Figma how far below the text it the line was intended to be. Can you provide a value or tell me how much to move it up?
  2. I can look at the shape again — I think the super long note at least is not representative of any actual content; I added it to see what would happen. I don't know about the short note. Could you suggest a minimum height for notes? (Or is there one in Figma?) Could/should we set a maximum height?
  3. Wow! 😞 I haven't seen those problems on the iPad I have been testing with or any of the desktop browsers resized to mobile. You said all the iOS browsers — can you share specifics (browsers, versions)?

@gwijthoff
Copy link
Contributor

Verified all of your testing notes above, everything seems to be working smoothly. This is such fantastic work, @rlskoeser!

Re: @gissoo note 2 above, I agree that we should think more about the case of long contextual notes. Many academic articles have footnotes/endnotes that go beyond the 82 words of that contextual note No. 5, especially when you factor in the addition of bibliographic references. One idea: maybe the user can scroll through the text within the contextual note, the same way I can scroll through the text I'm writing now in this little GitHub comment editing window?

And on @gissoo note 3 above, I'm also getting some funky behavior on iOS 13.4.1 using Firefox Version 26.0. I clicked contextual note 1, and then scrolled down on the article rather than clicking the 'X' to close the contextual note. The note disappeared but my place in the article jumped. And now when I go to click on that or any other contextual note, it won't open, and my place in the article moves.

Here's a screen recording of what continues to happen when I try to click on a contextual note again, even after I refresh the page.

@rlskoeser
Copy link
Contributor

@gwijthoff I think Nick also suggested this! Set a max height, but allow the text inside the note to scroll if it's longer than that. @gissoo how does this sound to you?

@rlskoeser
Copy link
Contributor

@thatbudakguy is it possible some of these problems are because I only overrode the click event for the close link? Do we also need a touch event handler?

@thatbudakguy
Copy link
Collaborator Author

not sure, but that sounds plausible!

@gissoo
Copy link
Contributor

gissoo commented Jun 11, 2020

@gissoo thanks for testing!

1. I couldn't figure out from Figma how far below the text it the line was intended to be. Can you provide a value or tell me how much to move it up?
  • @rlskoeser How useless of Figma, I'm sorry, it's 4-5px.
2. I can look at the shape again — I _think_ the super long note at least is not representative of any actual content; I added it to see what would happen. I don't know about the short note. Could you suggest a minimum height for notes? (Or is there one in Figma?) Could/should we set a maximum height?
  • @rlskoeser @gwijthoff @thatbudakguy Yes, there is a min height defined, this link should take you to the correct screen, I have mixed feelings about the usefulness of defining a max height. Yes, I recall the discussions on making it be scrollable, I had agreed with that. There is another approach, which is to define a max height and if the content runs longer than the max height then have a "...read the rest in the end note" ..or something of that sort ...it might help with the readability of the main article and helping users to focus.
3. Wow! 😞  I haven't seen those problems on the iPad I have been testing with or any of the desktop browsers resized to mobile. You said all the iOS browsers — can you share specifics (browsers, versions)?
  • @rlskoeser I mean FF: 26.0 (18099), Chrome: 83.0.4103.88, and Safari:13.4.1 – iOS 13.4.1. The first two (FF and Chrome) are all the latest, there is a new there is a 13.5.1 version of iOS I have not installed yet.

@rlskoeser
Copy link
Contributor

@gissoo I've updated with the following changes:

  • moved the gradient line up slightly (I'm actually not sure if knowing the placement in Figma would help me in this case the way I'm positioning it! But I will try to calculate if it's still not right)
  • Corrected the shape so it is based on absolute sizes rather than percentages (this should fix the angles that were wrong on the shorter and longer notes)
  • Configured max/min heights for the notes; if the content is longer it is scrollable within the note
  • Corrected the spacing at the bottom of the note

I also found and corrected one interaction problem on Safari.

@rlskoeser
Copy link
Contributor

@gissoo @thatbudakguy when I was revisiting the contextual notes styles, I noticed that the designs have a drop shadow (which I had forgotten — sorry). I tried to implement it, but apparently css box-shadow doesn't work on absolutely positioned elements (I tried several different approaches to try to get around this). I have one more idea about a way to implement this, but wanted to find out how important you think it is.

@thatbudakguy
Copy link
Collaborator Author

oh no, I could've saved you some time... last week I also realized it didn't have the shadow, tried to add it, came to the same conclusion and decided it wasn't worth it. I should've said something!!

@gissoo
Copy link
Contributor

gissoo commented Jun 16, 2020

@gissoo I've updated with the following changes:

Thanks for writing, @rlskoeser

* moved the gradient line up slightly (I'm actually not sure if knowing the placement in Figma would help me in this case the way I'm positioning it! But I will try to calculate if it's still not right)
  • This looks so much better, thanks!
* Corrected the shape so it is based on absolute sizes rather than percentages (this should fix the angles that were wrong on the shorter and longer notes)
  • This looks great!
* Configured max/min heights for the notes; if the content is longer it is scrollable within the note
  • @rlskoeser This is awesome! Looking at note no. 5. However it's hard to tell that there is more content to scroll, and also the text alignment is off (at the right getting cut off). I will attach a video about this on Slack so you can see. Can we maybe use a shadow where the cut off line is for the text inside the container to make it apparent that "there is more content to see by scrolling"? (also, sometimes the note no. 5 container's top is cut off, is there a way to have it always appear with its top fully visible?, feels like a bug or that it depends on y-axis of the reference link at the time it is selected)

Image from iOS (3) copy 3
Image from iOS (3) copy 4

* Corrected the spacing at the bottom of the note
  • @rlskoeser This is great, the only thing that still looks wrong is the short note, there is still too much space. The space from the bottom of the text to the bottom of the container should be 71px based on Figma (or just match the current spacing of the other notes). The other notes look right, Note no. 2 and 4 are the only ones that look wrong. Here is an image:
    Screen Shot 2020-06-16 at 11 17 36 AM

I also found and corrected one interaction problem on Safari.

  • The contextual notes are not showing up for me on iOS Safari and firefox (I am attaching screen recordings on Slack so you can see). The only one that works well is iOS Google Chrome.

@rlskoeser
Copy link
Contributor

@thatbudakguy wish I'd known! My one thought for another approach is to set the purple card shape on the ::after element and set the actual li to have a slightly larger width and height, and set borders or a background image to simulate the drop shadow — but that's a lot of effort, and it might require adjusting the placement of the notes.

@rlskoeser
Copy link
Contributor

@gwijthoff I changed the implementation for how notes are closed — could you test and see if this fixes the jumping behavior on mobile you documented earlier?

For anyone interested in the specifics: it's apparently difficult to change the document location hash without scrolling but still have the CSS ::target be updated. I was using a solution I found on Stack Overflow that seemed to work for desktop browsers, but it doesn't seem to work on mobile. I switched it to something that's actually simpler: setting the hash to a non-existent id. This updates the note so it is not targeted, but does not scroll the document.

@gissoo
Copy link
Contributor

gissoo commented Jun 16, 2020

@rlskoeser @thatbudakguy re the contextual note container drop shadow: I think it's fine for this first issue/release to not have the shadow. I totally missed this!
However, I think it adds a lot visually, the container is too flat right now. (compared to when there were some shadow texts inside and the drop shadow – the design)

Does the information in this link help by any chance? There are several points there, if you haven't looked at it yet https://stackoverflow.com/questions/16198745/css-box-shadow-covering-all-contained-divs-using-absolute-positioning/16199113#16199113

@rlskoeser
Copy link
Contributor

@gissoo somehow I calculated the min-height incorrectly! It seems that actually removing the min-height does the right thing. Would you confirm that note 2 looks correct now?

@gissoo
Copy link
Contributor

gissoo commented Jun 16, 2020

@rlskoeser the heights for note 2 and 4 look correct now, thank you

@gwijthoff
Copy link
Contributor

@rlskoeser oof, still getting some residual funkiness on mobile. Here's a recording, iOS 13.4.1 / Firefox 26.0 After I click a contextual note for the first time, subsequent contextual notes won't open on my first tap. A second tap does bring them up. Not sure that this matters, but the contextual note text also exceeds the right boundary of my screen. You'll see it in this video with the phrase "Universit/y of Minnesota Press."

Also, contextual notes with long text on mobile seem a little shy, as you'll see in that recording. I'm having trouble scrolling through the note: the second I does, it gets all bashful and hides.

But on a positive note, the notes do close properly now on mobile when a user scrolls down!

@rlskoeser
Copy link
Contributor

rlskoeser commented Jun 17, 2020

@gwijthoff thanks for flagging the content width issue on mobile! Fixing that corrected another problem I was seeing on some devices (specifically, scrolling long content within a note on FF).

@gwijthoff my latest updates have the following changes:

  • content width inside the note should now be set correctly
  • click/touch target for references is larger, should be somewhat easier to touch
  • added a touchend handler that I hope will resolve the iOS problems we're seeing (it seems to behave much better on the iPad I'm testing with, and I don't think it introduces any new problems)

@rlskoeser
Copy link
Contributor

@gissoo I think the max height for the largest note is still not correct, which might explain why it displays incorrectly on your phone. I added size constraints based on what I see in Figma to set the max height, but the full size of the container is larger than the max height in your designs! 🤯 I don't know if I'm doing some math wrong, or if the angle of the triangle at the top of the note is wrong, or what! Can you look at the largest note and suggest how or where you'd like me to adjust the max height size? The easiest thing for me to change to address this is the height of the text content inside the note. (But if I have the angles wrong, let's fix it!!)

@gwijthoff
Copy link
Contributor

Closing this monster issue, since most of it has been resolved. Any further contextual note problems can be referenced in new bug reports. Thanks for following up on all of the above, @rlskoeser!

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

No branches or pull requests

4 participants