-
Notifications
You must be signed in to change notification settings - Fork 23
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
Add an info tab + informative tooltips #157
Conversation
I am reopening this because it is good to have this in a separate pull request than #164. |
Add hover over instructions to features on home screen. (i.e. on test-corrections or download buttons) |
|
@andOrlando Thanks for the changes. Now that I see the custom tooltip, I actually think that it looks nicer than simply using the HTML Having tooltips reminds me that the icons in the header of the grades table are confusing because the "Add assignment" icon is a button while the other two (test corrections and statistics) are not, and because the per-assignment icons directly below the plus sign are for deleting assignments. We could make it so that each individual button (for each assignment) has a tooltip and do away with the decorative header icons (so that the plus sign is the only icon in the header). Your thoughts? I also noticed that there is quite a bit of boilerplate for each tooltip. There is a pure-CSS way to read from an attribute of an element (instead of having multiple elements), so we should be able to achieve the same styling with less HTML code (see https://stackoverflow.com/q/2011142 and https://jsfiddle.net/z42r2vv0/2/) by reading from the |
@psvenk Each individual button with a tooltip sounds cumbersome, whereas the green tint of the plus button makes it pretty clear what it does, and in my opinion it'd probably be fine just to let people figure out that it refres to the icon buttons. I will make the attribute changes asap, didn't know those things existed. Edit: Oops I thought the + was more green, I might change that in a separate commit and people can say what they think wednesday |
@andOrlando Instead of having an instantly appearing tooltip for each individual button, we could use a CSS transition to make the tooltips appear only after the user hovers over a button for more than, e.g., 1 second. Alternatively, changing the color of the statistics and test corrections header icons from green to a more neutral color like gray, combined with removing the tooltip from the plus button, could make things more clear and remove the need for tooltips on individual buttons. |
9395d16
to
c742b01
Compare
adds info tab with new formatting
- Wrap text at 80 characters (more readable than overlong lines) - Use <strong> instead of <b> - Use semantic HTML (<h2> for headings, <p> for paragraphs) - Use CSS margin-bottom instead of abusing <br> to add vertical space
adds hover spans to certain (mostly header) icons. Please fix my css if I messed something up, especially that whole .tabulator-col .tabulator-header thing.
updates header-icon and a couple other icon classes so that they're centered in their columns. This was my first commit without the handholding of github desktop :)
Fixed the click areas that I messed up last time Changes how the icons are centered, makes them actually be in the center of the header instead of just using left padding also adds a tooltip for the hide button also makes the plus button green (sorry psvenk for not doing as separate commit)
In accordance with the changes in commit b9dcae0.
Revise to increase clarity and fix grammatical errors.
I attempt to set up the tooltips as soon as the parents have an actual width so I can calculate what the margin-left should be
- Delay before and after clicking along with slight transition for opacity - removes header tooltips from some icons - adds tooltips to table icons - standardizes icon sizes to make it easier to hover/click - makes GPAType function only change the text and not the whole html of an element - updates setup_tooltip_margins to not do margins unless the width of its parent isn't 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, but let's hold off on merging for now until some other people take a look and comment on the length of the delay for the tooltips.
We have formed a consensus that 375ms is more discoverable than 750ms but still unobtrusive.
- fixes the said bug - reformats some of the toggle_fullscreen_pdf code for readability
Remove commented dead code and fix spacing of "for" loops
Closes #76. Closes #178. Please do fix any inconsistancies/things that go against convention and tell me about it becuase I haven't really done too much html. We can probably improve on this as people have ideas and with this tab we really do need a dropdown or something before we actually merge this as to not have way too many tabs.
Also I don't know how to collaborate on a branch so I'm just going to pull I guess