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.

Sign up for GitHub

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

Jump to bottom

Apidoc better search #10720

Merged
merged 28 commits into from
Feb 28, 2020
Merged

Apidoc better search #10720

merged 28 commits into from
Feb 28, 2020

Conversation

MoonE
Copy link
Contributor

@MoonE MoonE commented Feb 21, 2020 •
edited
Loading

Searching feels rather slow, and the result relevance is not always great.
I set out to fix this.
There were also a few bugs, that are fixed with this:

  • Clearing the input box after searching doesn't restore the original state instead there are the subtitles visible, and clicking the icon no longer expands members
  • Entering an invalid regexp throws an exception
  • When hovering a class entry the cursor changes to indicate it can be expanded, but actually only the glyph icon is clickable
  • when searching for a class name all members are also matched, but not events because the search is done on the longname of each item / member, and events belong to different classes; searching for 'module' matches everything
  • some entries are displayed with a wrong name e.g ol/interaction/DragBox~DragBoxEvent is also listed as ol/interaction/DragBox
  • the keyup event used on the search input also captures keys like CTRL and arrow keys
  • some events like e.g. boxdrag are not highlighted when clicked because a jquery error is thrown

Besides making searching faster and showing more relevant results I also did some small improvements (imho):

  • If the search box is not empty on page load a search is started to show the corresponding results, Firefox leaves the input intact when reloading a page / going back to a previous page
  • The prettyname includes the class name if it is different from the module name ol/source/Vector~VectorSource instead of only ol/source/Vector
  • To better display the longer names a zero-width space is inserted before '~' and '.' in class and module names to allow word wrapping when needed
  • The search is only done on the string that is displayed to the user - that's the prettyname for classes / modules and the name for members / events.
  • The icon and class name are displayed in a two-column flexbox layout so wrapped lines are indented nicely
  • When search results are displayed clicking a class toggles between the matched members and all members instead of collapsing all

Results are more relevant when

  • any word of the path starts with the search term +
  • the actual short class / module name starts with the search term ++
  • the start and end of the name are both at word boundaries +++
  • the whole name matches from start to finish ++++

additionally there is a small preference to

  • shorter names
  • classes over modules

The speed improvements are achieved by

  • not using jquery with each item in the search loop
  • keeping track of the matched items from the previous search

Feel free to try it and leave a comment.

@MoonE
Highlight current anchor with css instead of js.
f5a996a 
The `:target` selector is supported by all browsers.
@MoonE
Hide member-lists, show current item when clearing input
6187118
@MoonE
Skip searches when typing fast / slow cpu.
b919074
@MoonE
Provide the searchTerm as argument to doSearch function
99ecce2
@MoonE
Add code to measure search function speed
d56513b
@MoonE
Catch errors when search term is an invalid regular expression
2b8582f 
On error search escape special characters, with only '.' matching
any characters.
@MoonE
Remove unnecessary / duplicate code.
b9455bf
@MoonE
Store search data in lowercase and compare in lowercase
f7871d6 
This way we can do the search case-sensitive after only converting
the search term to lowercase.
@MoonE
Search if the search input is not empty after page loads
d3b9b25 
Firefox remembers input on the same page if a normal reload is performed.
In this case the results for the search term should be shown automatically.
@MoonE
Only match the displayed name of items when searching
7640bcd 
Currently the search term is matched against the longname of members.
Because of this most members are matched when searching for a
class / module and also searching for 'mod' matches everything as
all longnames start with 'module:'
@MoonE
Change prettyname of classes
8b857ee 
Now also includes the class name if it is different than its module name.
`ol/source/Vector` -> `ol/source/Vector~VectorSource`.
@MoonE
Allow word wrap before '.' and '~' in class names
cc561da
@MoonE
Indent line-wrapped class names
1b4ed08
@MoonE
Improved weight function.
ccafb4b 
++++ Full match from start to end
+++  Full match between word boundaries
++   Begin matches at word boundary, multiple matches are better
+    Matches somewhere

Classes with (multiple) members matching at the beginning are
weighted higher.
@MoonE
Fix regex dot escape
b613e8e
@MoonE
Put more weight on the base name
15beb1f
@MoonE
Use strict mode
99d9a9f
@MoonE
Weight function prefer classes over modules
646ecfd
@MoonE
Use css to show / hide items; track matched items
2dec296 
... to improve search speed.

Instead of marking all items as hidden, and then marking the matched
ones again, this keeps track of all matched items and only
removes those that no longer match and adds those that previously did
not match.
@MoonE
On toggle when searching show unfiltered instead of hiding all
b46b250
@MoonE
Shorter css paths in navigation
ee8ec6f
@MoonE
Allow toggel on whole title, add toggle transition
bf2a7bd
@MoonE
Avoid deprecated jquery :eq selector
df8c0f3
ahocevar
ahocevar reviewed Feb 27, 2020
View reviewed changes
Copy link
Member

@ahocevar ahocevar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work on this @MoonE. I like everything except for

The prettyname includes the class name if it is different from the module name ol/source/Vector~VectorSource instead of only ol/source/Vector

This is because the class name is irrelevant - the classes are the default export.

@MoonE
Copy link
Contributor Author

MoonE commented Feb 27, 2020

  • some entries are displayed with a wrong name e.g ol/interaction/DragBox~DragBoxEvent is also listed as ol/interaction/DragBox

This was part of the reason why I changed it.

I now changed it to only keep the class name for non-default exports. So
ol/interaction/DragBox~DragBoxEvent is still listed like this, but it is ol/source/Vector for both module and class instead of ol/source/Vector~VectorSource for the class.

@MoonE
Some more cleanup
e484eca 
No need to sort beforehand and therefore sorting can be done at last too.
Also forEach is just fine, no need for underscore.

This changes the navigation default order for modules to be after classes
when the name is the same.
ahocevar
ahocevar approved these changes Feb 28, 2020
View reviewed changes
Copy link
Member

@ahocevar ahocevar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @MoonE for this great improvement!

@ahocevar ahocevar merged commit 6370175 into openlayers:master Feb 28, 2020
@MoonE MoonE deleted the apidoc-better-search branch February 28, 2020 21:21
@snyk-bot snyk-bot mentioned this pull request Apr 25, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahocevar ahocevar ahocevar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@MoonE @ahocevar