Requesting JavaScript code review for new glossary popovers feature#4285
Requesting JavaScript code review for new glossary popovers feature#4285micshar92 wants to merge 12 commits intoopenboxes:feature/upgrade-to-grails-3.3.10from micshar92:feature/upgrade-to-grails-3.3.10
Conversation
jmiranda
requested review from
alannadolny,
awalkowiak,
drodzewicz and
kchelstowski
|
@micshar92 Apologies for the delay. I probably won't have a chance to review this week but have requested a review by other developers. I will take a look next week. |
awalkowiak
left a comment
awalkowiak
left a comment
There was a problem hiding this comment.
@micshar92 the pages that you want to add are markdown, and you are trying to squeeze in html and jquery. I am not sure if this makes sense. Here is how you can achieve tooltips in markdown: BoostIO/BoostNote-Legacy#3579 (comment) (not a very good-looking version of tooltips, but works).
|
@micshar92 here is an example of "tooltip" for bin location: Lorem ipsum [bin location](## "Bin location
A location that designates a particular shelf, pallet position, or bin within a given zone in a given depot. For example, within Zone A, you may have a bin location for pharmacy items.") something somethingAnd how it looks like: |
|
@awalkowiak I briefly considered the HTML Still, I think it's best to populate the terms from an external source file. Markdown isn't built for huge amounts of code inside Maybe I'm trying to do something too complex. It seems simple in theory to me, just hard to actually write the code. Is it actually impossible? |
|
@micshar92 my example is still a link, but I have not used the link part in it, so it can be added. I am still not sure about using HTML and jQuery when the convention (other docs here in the project) is using a markdown. @jmiranda what are your thoughts about this? |
|
@awalkowiak Yeah, you're right. Scrap the HTML idea. As for the tooltip you showed... I can compromise on it. To me it's close-to-perfect, where the popover would be perfect. BUT maybe that's just me wanting my idea. It seems to carry the same functions, and it's drastically easier. Not all of the terms have links, though. I'm thinking a simple style change: make the underline dotted for no-click terms, make all terms a regardless of type cadet blue. If my original idea is impossible, we now have a Plan B. EDIT: Ah, wait. But if a term changes or is deleted or added, every page with that term would be instantly outdated. But maybe that's not a huge issue. |
|
@awalkowiak @jmiranda How about this as a compromise: since I have little desire to learn JavaScript, and because I don't think a definition change will take that long to fix or be that hard (ctrl-F), let's drop the JSON/JavaScript idea. Instead, I'll go purely with HTML and SCSS. I saw this screenshot in the recent release notes. I'll just copy the style you use here, and add
Tell me how you made these, and I'll use the same style for the terms, if it's possible. |
|
@micshar92 these pages are made in React, and for these particular popups, we used this package: https://www.npmjs.com/package/react-tippy. I have a question though - was the HTML + CSS requested here to be used instead of a simple markdown? |
|
@awalkowiak Markdown lets you use That React package looks perfect. I'll use it. |
|
(React doesn't appear to work with Markdown, but...) 
... and after hover.
...but then I realized there's no opportunity for me to use this. Why did I make this? Outside of the PiH pages or the wiki-- neither repo of which I can edit-- there are no terms to use a tooltip with on any page. Anyway, I did still make the glossary page, but it's like the garnish on a fancy dish and I don't expect it to get much traffic. Oh well. If you choose to accept this super simple thing, here you go. Looking forward to something more substantial, like what @jmiranda and I talked about in #4235 with Postman. OpenBoxes is directly in my writing niche, so I'll stick around. |
|
Looks good @micshar92. FWIW, the following mkdocs plugin (https://github.com/AngryMane/mkdocs-glossary-plugin) might be useful. It doesn't solve the problem but gets us close. When a page is loaded, the mkdocs glossary plugin will iterate over the glossary terms defined in the glossary directory
and attempt to find those glossary terms within the page. When it finds one, it annotates (wraps) the term with a link to the glossary term.
And that link leads us to an individual page with the glossary term definition.
This isn't exactly what we want but it gets us part of the way by allowing us to
So we can either extend the plugin to allow for a link vs tooltip option. This might be more difficult than I'm imagining but I thought this was cool. The other bummer about this plugin is that we need to maintain a separate file per glossary term instead of a JSON file with everything defined. But it solves the one problem that I expected to be difficult i.e. automating the application of glossary terms in the DOM. So perhaps it buys us enough to work with. |
|
The mkdocs Annotations plugin (https://squidfunk.github.io/mkdocs-material/reference/annotations/) also looks promising but requires the annotations to be included in the source markdown content.
Not sure if mkdocs has a pre-process/post-process hooks, but if so we could potentially rewrite the markdown content during the deployment process OR perhaps use the two plugins together. |
|
Here's another potential solution. |
docs/support/glossary.md
Outdated
| <dt>ABC Analysis class</dt> | ||
| <dd>This class system categorizes products based on their impact on cost. Products in Class A contribute to the majority of costs because you use and buy them most often. You buy Class B products more infrequently. Class C items are not high value and do not need many follow-ups.</dd> | ||
|
|
||
| <br> |
There was a problem hiding this comment.
You can remove the <br/> elements as we can add space using CSS.
There was a problem hiding this comment.
Done. You're right: in my typography notes, I wrote down long ago to never use carriage returns. Looks like I forgot this time. Good lesson to learn before making that mistake for a big company.
3 participants
Hi, everyone. I'm a junior technical writer volunteering here to beef up my portfolio. My background is in supply chain and data, and I want to prove my ability in APIs and code. That said, while I understand Python, SQL, and markup languages, JavaScript is proving existentially frustrating.
I'm requesting a JS code review for my glossary popovers feature. The idea is to have an
<a>that when hovered over shows the term and definition (bootstrap popover title and content respectively) fetched from the glossary.json file. Wikipedia and the GitHub docs have something similar. Mine, however, show up like this:Where
bin_lTermis supposed to be "bin location" andbin_lDefis supposed to be its definition, as I declare earlier in the JS. Here's an example of what I'm trying to do:I might be on the right track, I just can't implement it correctly. Forgive me; I don't think I have a developer's brain.
My idea is:
<a href="LINK">ARTICLE</a>" if the term links to a tutorialBut here are the problems I know of:
trigger="focus hover"work? It only works on click.fetch()functions. I'm also not sure about the concatenation for definitions that include an HTML link after it.Please review / help. Thanks!
(Note: glossary.md is only populated with two sample terms as of now. I didn't want to write every term if the code for them was wrong.)
(Note 2: Once this is all correct, I'll get to work on finding terms across the site's guide pages and making them into the popover class. I think that's right.)