Skip to content
This repository has been archived by the owner on May 30, 2019. It is now read-only.

custom tooltips left unimplemented #480

Open
yawitz opened this issue Jan 21, 2015 · 25 comments
Open

custom tooltips left unimplemented #480

yawitz opened this issue Jan 21, 2015 · 25 comments

Comments

@yawitz
Copy link

yawitz commented Jan 21, 2015

Due to a variety of issues with the code libraries we're using in Samplestack, implementation of tooltips for a number of features are blocked. From @clockworked247:

  • Paging Control - with the new implementation of pagination using the new angular-ui bootstrap directive does not allow us to hang tooltips on the individual numbers in the pagination control.
  • Markdown Editor - when tooltips are attached to the editor buttons, a new unintended behavior is added to those buttons on hover. Irritatingly, the button border-right size appears to change, moving buttons to the right by one pixel. This then fades out after a second. The effect seen when hovering over the buttons is that the layout completely shifts and slides back.

Recommend implementing all except these two sets, as tracked in #473, and deferring these remaining two to 8.0-2.

@yawitz
Copy link
Author

yawitz commented Jan 21, 2015

NOTE: For both this issue and #479, Scott says:

For the purposes of the implementation of the tooltips, I am separating the issues out because the work needed to address this could involve the use of a new library, forking of the existing one to add new features, etc.

@clockworked247
Copy link
Contributor

@yawitz I posed a question in #479 to see if we just want to stick with browser based tooltips.

If we do, fortunately, the browser based tooltips are already in place for the Markdown Editor buttons. So that's good. If we want to find a different library or write our own directive we'd have to ensure the same side-effect does not occur (moving buttons on rollover).

For the Paging Control directive, there's no way to add tooltips for those, regardless of how we decide to implement tooltips. The directive for the paging is completely self contained, so we cannot manipulate the HTML that it creates and add tooltips on it. So adding tooltips here will not be possible.

@clockworked247
Copy link
Contributor

One correction on the Paging control tooltips. @laurelnaiad pointed out that yes, it is technically possible to do anything we want with JavaScript. So we could add JS code to achieve this.

So to amend my comment, this is true, but the code would be very complex in that it would have to attempt to inject tooltips into the directive's HTML, monitor and watch when that HTML changes due to user interaction, and update it after the fact... which is not best practice and is a heck of a lot of work for adding tooltips. There are other alternatives to this implementation (like rewriting the directive), but this is equally challenging if we go with an a non-browser based tooltip.

All of this is to say that it is possible but the ROI seems quite low.

@yawitz
Copy link
Author

yawitz commented Feb 23, 2015

I reviewed this issue (and #479) with Kasey. Long story short, we will want to fix things like this for our apps going forward (in whatever framework we settle on, whether Bootstrap or something else). Whether we choose to fix this in Samplestack short-term is a prioritization question that should be addressed in our upcoming triage meeting.

@popzip
Copy link
Contributor

popzip commented Feb 23, 2015

We need a bit more info to make a determination on the various tooltips work -

  • What are the different issues (Layout/wrapping, paging control, markdown)
  • Which are IE9-specific and which are not? (I think we are very close to de-supporting IE9 for Samplestack)

Project priorities that are relevant to this topic are: 1) To use modern development patterns & design 2) Show typical features in a modern MarkLogic app.

If we are to abandon the design as spec'd (with black formatting), we need to be able to articulate why it can't be done. For example if our choice of libraries/technologies: Bootstrap+Angular+Paging Control cannot be done under any circumstances, I'd like to be able to articulate that very clearly/confidently: "While bootstrap was our overall framework, we abandoned it in XYZ because it won't work, requires significant customization, etc". If these are common libraries and patterns, I'd expect a pointer to an external bug or other 'proof' (consensus in the community - blogposts, Stack Overflow qs) that it can't be done.

I think we need a bit more digging to 'prove' why our architectural choices don't allow for this feature OR to explain what the discrepancy is and what the work effort to resolve is, so we can make a trade-off call on whether its worth putting the time in during Samplestack.

@clockworked247
Copy link
Contributor

@popzip

What are the different issues (Layout/wrapping, paging control, markdown)

The three you mention here are, AFAIK, the only UI issues we ran into with the use of Angular Bootstrap tooltips.

Layout - lack of fine grain control over the layout of the width and heigh of the tooltips, even through CSS.

Paging control - The paging control is a directive that replaces it's minimalistic HTML tag on the page with fully functioning pagination controls. This happens in JavaScript through the directive itself and renders depending on user interaction. There is no built-in affordance to attach tooltips to the individual numbers or to the arrows on controls. Normally you'd just add a property to the HTML to setup the tooltip. With the pagination control directive, the HTML is generated dynamically, so there is no practical way to add these tooltips.

Markdown - I don't believe this is markdown specific, but related to the way the buttons in the Markdown editor were rendered side by side. When a button was hovered over, that button's effective width changed size. This caused the UI to move as the user hovered over the buttons.

Which are IE9-specific and which are not? (I think we are very close to de-supporting IE9 for Samplestack)

I'm not sure the tooltips issue is going to be the catalyst for elimination for IE9 support (now currently only holding 1.4 % of the entire browser market).

The UI manifestations as listed above do not relate to why IE9 failed. The use of the library itself and how it interacted with Angular's digest cycle caused errors within Angular on IE9. That's bad, very bad. It was only when we completely removed all tooltips, did we see IE9 tests begin to once again pass. So what I'm saying is that the Bootstrap tooltip library itself has a flaw which does not appear to relate to a particular use (as observed in my limited investigation pre-1.0).

If we want to have style-able non-browser default tooltips, then we would need to either roll our own or leverage another open source UI library that implements HTML tooltips. I know @laurelnaiad has been wanting to replace the use of Bootstrap for some time. Maybe the alternative would provide us with tooltip directive that does not suffer from the above problems. This would require some investigation.

Please let me know if there's any more information you need from me.

@popzip
Copy link
Contributor

popzip commented Feb 24, 2015

Thanks for the context.

Let's assume for the sake of this discussion that IE9 is no longer in the picture (not because of tooltips but for other reasons).

Do all three of these issues still exist?

Can you point to evidence outside of Samplestack that highlight what particular components don't work together? Is it Angular + Bootstrap for tooltips that is a known incompatibility?

@laurelnaiad
Copy link
Contributor

It's my belief that the level of effort being expended, and the extra code being delivered in our repository which can distract from the important code, are both antithetical to our primary objective: teaching how to build applications on MarkLogic.

Indeed, running around in circles to support IE9 is in that category, and use of Bootstrap generally (which clutters the HTML) is in that category, though those subjects are orthogonal and larger than just the tooltips issue. I really think we need to keep our eye on the ball with building a teaching app where what we're teaching is how to build MarkLogic-based software, not how to deal with dated browsers or make fancy tooltips.

@laurelnaiad
Copy link
Contributor

Can you point to evidence outside of Samplestack that highlight what particular components don't work together? Is it Angular + Bootstrap for tooltips that is a known incompatibility?

The real reason to consider not using Bootstrap is that it obscures our mission due to the HTML pollution it requires. Semantic-styling frameworks would simplify our HTML. Is it worth it to change in v1.1.0? No. Should we think about it for later? I think so.

@clockworked247
Copy link
Contributor

Let's assume for the sake of this discussion that IE9 is no longer in the picture (not because of tooltips but for other reasons). Do all three of these issues still exist?

Yes, all three still exist with the Angular Bootstrap UI library. The IE9 issue only has relevance in the context of test breakages.

Can you point to evidence outside of Samplestack (?)

I cannot. Like any software, it has bugs. So it's possible that the authors could fix the issue. Since we were right at the deadline, I didn't file an issue with them (it'd take some time to craft an isolated case). We could file one, but there are no guarantees they would fix it, especially given its an edge case that involves IE9 only observed while running tests.

@yawitz
Copy link
Author

yawitz commented Feb 24, 2015

We're still also talking about the non-IE9-specific issues, such as the button shifting in the markdown editor buttons, and the positioning problems for tooltips that fall near the edges of the window, right? Those don't seem like edge cases, or in any way related to the testing failures.

@clockworked247
Copy link
Contributor

@yawitz - Correct. We should talk about them separately, to avoid any more confusion.

@clockworked247
Copy link
Contributor

@popzip I managed to find an open issues relating to the problem we discovered with tooltips causing digest cycle problems. See:
angular-ui/bootstrap#516
The summary of the issue is: "Tooltip throws exceptions if bound to button and ngClick takes long"

For us, the container that the logout button lived in had a tooltip on it. When that button was clicked, on occasion, we would see a similar error.

I hope that is what you're looking for. If you need something else, please let me know.

What's the way forward with this bug?

@yawitz
Copy link
Author

yawitz commented Feb 25, 2015

Any references to the layout problems we saw (i.e. that tooltips near screen edges would be positioned partially offscreen, rather than fully within the window boundaries)?

@clockworked247
Copy link
Contributor

Best I could find were:

Cut-off Tooltips on Edge of screen
angular-ui/bootstrap#2990 (closed, but people are still complaining)
angular-ui/bootstrap#432 (closed, but people are still complaining)

RFE to all custom CSS classes
angular-ui/bootstrap#3126 (we were wanting to use CSS to try to control width styling, which didn't work. This RFE requests custom CSS classes for the tooltips that may help with that issue).

All in all, I'm seeing updates to the library since January 2014. I'm not sure how thorough they are being in their clean-up of issues, as it seems some issues are being closed before actually fixing issues and people are complaining.

Let me know if either of you need anything else.

@yawitz
Copy link
Author

yawitz commented Feb 25, 2015

That's helpful, thanks Scott. Interesting to discover that the angular bootstrap version of this doesn't match the behavior of bootstrap classic. I think this gives us some cover if/when developers ask us about it.

@popzip
Copy link
Contributor

popzip commented Feb 25, 2015

This is great info - thanks for digging Scott. I'm all clear on the trade-offs and why the effort would be required to make this work. I agree its not worth the investment. I'm good with closing - only question from me is to Mitch whether you want to update wireframes or let this conversation in GitHub be system of record. We can close this issue (not defer).

@popzip
Copy link
Contributor

popzip commented Feb 25, 2015

Same resolution for #479.

@yawitz
Copy link
Author

yawitz commented Feb 25, 2015

The wireframes are not meant to depict visual design, and we agreed we would not try and keep Stephen's mockups up-to-date, so I think these threads, and the implementations that get approved, should be sufficient to record this decision.

The only thing left, I think, is to update the tooltip table to reflect the tips we will not be supporting. Scott, given how sprawling these threads have gotten, perhaps you can post a final, quick summary of which tips are being dropped, and I can update the table accordingly.

@laurelnaiad
Copy link
Contributor

Why don't we leave this as an open bug that's not assigned to a milestone nor a team member?

@clockworked247
Copy link
Contributor

I'm all for closing these tooltips related bugs and moving on.

@yawitz I believe the only tooltips we weren't able to implement using browser-based was the pagination on the QnA search list. I'll do another double check just to be sure and post it here.

@laurelnaiad laurelnaiad removed the major label Feb 26, 2015
@laurelnaiad laurelnaiad removed this from the 8.0-2 milestone Feb 26, 2015
@laurelnaiad laurelnaiad changed the title Tooltips blocked for a number of features custom tooltips left unimplemented Feb 26, 2015
@popzip
Copy link
Contributor

popzip commented Feb 26, 2015

i don't think there's a reason to leave open. if it becomes a priority in the future a new requirement/RFE can be created. this shouldn't just hang out in the backlog.

@laurelnaiad laurelnaiad added external and removed fix labels Feb 26, 2015
@clockworked247
Copy link
Contributor

Hey gang. Sorry for the wait. I de-prioritized the last check of the tooltips yesterday in favor of making some solid progress on the Node.js middle tier.

After a thorough check today, the only places we have not implemented tooltips are the pagination and the related tags (which is in the process of being implemented). I've asked @wooldridge to add the related tags tooltips as a part of his work, so when he completes that, it'll be in.

@yawitz
Copy link
Author

yawitz commented Feb 27, 2015

OK, I updated the table in the Wiki Tooltips table with a note that the pagination control tooltips are deferred until further notice.

@yawitz
Copy link
Author

yawitz commented Mar 3, 2015

Note that this overall issue should also include any tooltip details covered in #479 (which has been closed).

@popzip popzip modified the milestone: backlog Mar 3, 2015
@grechaw grechaw removed this from the backlog milestone Mar 10, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants