gleam icon indicating copy to clipboard operation
gleam copied to clipboard

Add built-in search to generated documentation

Open lucasavila00 opened this issue 1 year ago • 19 comments

Closes #1634

It works, styling is not finished though. A demo is live https://deploy-gleam-docs.web.app/

I wonder how we should do the mobile version of it...

Please pardon the horrible rust code, I hadn't used rust in 5 years.

TODO:

  • [x] keyboard navigation isn't working
  • [x] mobile layout
  • [x] desktop layout
  • [x] suggestion lines are not wrapping, content is hidden if overflow is not 'hidden'
  • [x] support absolute URLS (ie: github pages hosting)
  • [x] escape HTML of search index in rust, not in JS
  • [x] svg icon colors are bad
  • [x] css is using hardcoded sizes, not the variables
  • [x] js code style is inconsistent with the current JS code
  • [x] search index json file should have md5 hash/timestamp in name, to avoid incorrect caching
  • [x] no prettier CI job
  • [x] no package.json
  • [x] add common synonyms to search

lucasavila00 avatar Aug 07 '22 02:08 lucasavila00

This is awesome and will help with discover and learning by docs a lot especially while more LSP work is underway.

inoas avatar Aug 07 '22 10:08 inoas

This is really cool! I am loving the demo. 💜

Could we remove the dependency on node modules please? It is nice to have the code formatted but I think currently mandating it adds a bit too much friction to the workflow for new contributors.

There's already quite a lot of things that need to be installed and I want to keep it as easy as possible.

Is there a way to make it so that string.append comes up first if you search for that or string append?

lpil avatar Aug 07 '22 11:08 lpil

Could we remove the dependency on node modules please? It is nice to have the code formatted but I think currently mandating it adds a bit too much friction to the workflow for new contributors.

Will do.

Is there a way to make it so that string.append comes up first if you search for that or string append?

Yeah. It doesn't work now because there is no string.append anywhere in the codebase. But we can add {module_name}.{export_name} to the search index, thus making it work.

lucasavila00 avatar Aug 07 '22 11:08 lucasavila00

I think it is ready to be reviewed once I fix the lint issues.

There is one TODO left, which I think should be done in another MR:

"properly parse markdown and remove it's syntax (not the replace approach)"

It will require adding a new dependency https://github.com/kivikakk/comrak and a custom printer. Maybe it is better not to do it?

lucasavila00 avatar Aug 07 '22 15:08 lucasavila00

Also, I did not document that there is a new setting in gleam.toml, a "homepage" that configures the deployment URL. It is required if the documentation is not at the root of the domain. It happens when deploying to github pages, for instance.

lucasavila00 avatar Aug 07 '22 16:08 lucasavila00

Looking really great! ✨

On the mobile mode when I enter text and then delete it the search input is de-focused and I have to click it again. Could we keep it focused?

The desktop search input looks a little off to me at the moment. There's not enough margin and too much padding on the left of the input, and the magnifying glass doesn't align with anything.

Perhaps the magnifying glass should go on the right side? Or maybe we could use the mobile design on all viewport sizes.

Also, I did not document that there is a new setting in gleam.toml, a "homepage" that configures the deployment URL. It is required if the documentation is not at the root of the domain. It happens when deploying to github pages, for instance.

We cannot do this via config as the documentation can be hosted anywhere, or opened from the file system. All URLs are currently relative and the search would need to work this way too.

lpil avatar Aug 07 '22 17:08 lpil

We cannot do this via config as the documentation can be hosted anywhere, or opened from the file system. All URLs are currently relative and the search would need to work this way too.

It is impossible to create a link to it, if hosted on github pages. A relative link on a page https://lucasavila00.github.io/sql-select-ts/ will delete the project name suffix from the link.

This setting is not mandatory, the default value for it makes the compiler use relative URLs. However, if one wants to use github pages to host the site, they will need to use a custom domain or this setting.

lucasavila00 avatar Aug 07 '22 17:08 lucasavila00

Perhaps "homepage" setting should be called baseurl, as in JTD/Jekyll? https://github.com/just-the-docs/just-the-docs/blob/main/_config.yml#L18

lucasavila00 avatar Aug 07 '22 17:08 lucasavila00

On the mobile mode when I enter text and then delete it the search input is de-focused and I have to click it again. Could we keep it focused?

That's fixed.

The desktop search input looks a little off to me at the moment. There's not enough margin and too much padding on the left of the input, and the magnifying glass doesn't align with anything.

Perhaps the magnifying glass should go on the right side? Or maybe we could use the mobile design on all viewport sizes.

I like the idea of using the mobile design on all viewport sizes. Less code to maintain. But I'm not sure if having it be fullscreen would be the best UX :thinking:

Notice that the lack of margin on the input is due to the fact the search input tries to match the width and position of the rendered content.

lucasavila00 avatar Aug 07 '22 17:08 lucasavila00

Notice that the lack of margin on the input is due to the fact the search input tries to match the width and position of the rendered content.

I'd not be concerned about that too much.

mobile:

  • looks great

desktop:

  • here long lib and version names will break the current layout and/or shorten the library name with elipsis, i'd flexbox align right a button on the max-width of sidebar+content with the spotlight/search icon and Search LIBNAME and on hit replace the whole header with a search bar

alternatively something like this:

Screen Shot 2022-08-07 at 21 29 34

inoas avatar Aug 07 '22 19:08 inoas

Notice that the lack of margin on the input is due to the fact the search input tries to match the width and position of the rendered content.

Sorry, I'm not sure what you mean there.

Here's a video that might show what I mean

https://user-images.githubusercontent.com/6134406/183759579-74e72d6d-efa0-43f3-8652-1ea4d73bb913.mov

When not focused there is extra padding to the left of the icon, while it looks correct when focused.

To the left of the input as a whole there is not enough margin to match the earlier gaps in the navbar.

lpil avatar Aug 09 '22 20:08 lpil

When not focused there is extra padding to the left of the icon, while it looks correct when focused.

Yeah, I'm aware. I was mostly worried about the general design. I'll make this padding go away.

To the left of the input as a whole there is not enough margin to match the earlier gaps in the navbar.

That's what I meant.There is no "margin", the design uses these "columns". We could increase the padding on the left column of the navbar, but it will make gleam_std name appear with ellipsis.

image

Personally I think the sidebar width should be increased, padding added to the left column of the navbar, the font of gleam_stdlib reduced, and the magnifying glass icon static to the left - but I didn't do it at first as it'd be a big re-design of the current site. I'm not good with design and the current one looks very good :p

lucasavila00 avatar Aug 09 '22 21:08 lucasavila00

We cannot constrain the first part of the titlebar to the width of the navbar, even on the largest size it is too small to fit a fair normal package name at the moment, such as with the stdlib.

image

I think having the search box aligned to the right or to use the full width like on mobile may be the best options.

lpil avatar Aug 10 '22 19:08 lpil

We cannot constrain the first part of the titlebar to the width of the navbar, even on the largest size it is too small to fit a fair normal package name at the moment, such as with the stdlib.

image

I think having the search box aligned to the right or to use the full width like on mobile may be the best options.

Ok. I think we should remove all desktop styling and have it be un-responsive, mobile all the time.

lucasavila00 avatar Aug 10 '22 19:08 lucasavila00

I did not make it use the mobile version, but a different approach to maintain the desktop one.

It's not perfect but might be good enough?

The demo was updated, one can see it if they refresh without cache

lucasavila00 avatar Aug 10 '22 20:08 lucasavila00

Screen Shot 2022-08-11 at 10 28 35

I agree that on desktop the sidebar should be larger, it should also be collapsable, and should probably collapse when searching, that way small width desktop isn't affected.

But personally I am still fine with the change as is, the gain is much larger with the PR as is and responsive layout styling optimization can be done in followup PRs by you, others (or me).

inoas avatar Aug 11 '22 08:08 inoas

I agree that on desktop the sidebar should be larger, it should also be collapsable, and should probably collapse when searching, that way small width desktop isn't affected.

Thanks for the picture, that collapsing of the name hadn't happened to me. Collapsing it is a good idea :thinking:

lucasavila00 avatar Aug 11 '22 13:08 lucasavila00

By the way, I wouldn't mind if anybody were to take it over from here :)

lucasavila00 avatar Aug 11 '22 22:08 lucasavila00

Roger. I will pick this one up to spare you the CSS bikeshedding 👍 Thanks @lucasavila00 !! Love this addition

lpil avatar Aug 12 '22 15:08 lpil