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.

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: sort on function names in the docs #6526

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

gunnarahlberg
Copy link
Contributor

The sorting of the entries is added, with styling that mimics neighbours.

Attributes are sorted by javasrcipts native comparison. Example: Str concat and contains are after UTF8ByteProblem and UTF8Problem

This is related to the #6494

I saw the list in the ticket. This PR ticks some of the boxes
Screen Shot 2024-02-15 at 17 26 11

@Anton-4 Anton-4 self-assigned this Feb 16, 2024
Copy link
Collaborator

@Anton-4 Anton-4 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 you contribution @gunnarahlberg, I like where this is going!

I failed to specify this in the issue but I think the sort button should be placed above Utf8ByteProblem (and under Str). This would follow UX best principles to put controls close to where they affect change. Otherwise if you're looking at the list for Inspect you are very likely to miss the Sort button all the way at the top.

Can you also try to make the Sort button stand out from the other items in the list? It's functionality is very different compared to the other links/buttons so we should visually indicate that.

@gunnarahlberg
Copy link
Contributor Author

100%

I was thinking something similar to what you said in your comment
Will fix

What about the cargo/rust stuff? It's literally my first time I do anything in Rust, all help is welcome. Should there be specific js files per feature?

@Anton-4
Copy link
Collaborator

Anton-4 commented Feb 16, 2024

I was thinking something similar to what you said in your comment
Will fix

Awesome, thank you @gunnarahlberg :)

What about the cargo/rust stuff? It's literally my first time I do anything in Rust, all help is welcome.

That all looks good!

Should there be specific js files per feature?

Good point! Having separate files probably leads to a slight additional delay per file. Feel free to combine both in script.js

@gunnarahlberg
Copy link
Contributor Author

@Anton-4 - I suggest that we instead of one sort button per section have just one sort button
Otherwise, this happens :)

Screen Shot 2024-02-19 at 22 44 19

What do you think? Would it be ok with you if the sort button happens once and sorts all the sections?

Like this

image

@Anton-4
Copy link
Collaborator

Anton-4 commented Feb 20, 2024

Otherwise, this happens :)

Hmm, is there no way to only show the sort button in the expanded view, after you've clicked on e.g. Str?

@gunnarahlberg
Copy link
Contributor Author

Oh, good idea, I didn't think about that before, let's try it out wouldn't be much trouble

The sorting of the entries is added, with styling that mimics neighbours.

Attributes are sorted by javasrcipts native comparison.
Example:  Str concat and contains are after UTF8ByteProblem and UTF8Problem

This is related to the roc-lang#6494
The toolbar has a search box and a sort button

Changed the presentation for the search input
@lukewilliamboswell
Copy link
Collaborator

@gunnarahlberg do you intend to change the behaviour so that Sort is only displayed in the expanded view? We might need to detect when links under class="sidebar-sub-entries" are hidden to do this.

Also, I think there are some formatting things for CI.

Otherwise this is looking really useful. 👍

@gunnarahlberg
Copy link
Contributor Author

@gunnarahlberg do you intend to change the behaviour so that Sort is only displayed in the expanded view? We might need to detect when links under class="sidebar-sub-entries" are hidden to do this.

Also, I think there are some formatting things for CI.

Otherwise this is looking really useful. 👍

Yes there is a change needed like that, I’ve been on vacation for a while

honestly I got it to work but not to be pretty 😥 e.g. it always looks wrong no matter how I place it

@gunnarahlberg
Copy link
Contributor Author

Ok i have this idea that might work

We show a sort button to the right of the section title when the section is expanded

Some of the font awesome icons are used in this docs. Maybe this icon works, what do you think? arrow down a-z the classic solid is free others are nicer but requires a license

@rtfeldman
Copy link
Contributor

Regarding icons and licenses, my preference is (and I know we aren't doing this consistently, but I'm working on changing the other place we use an icon):

  • Only use SVGs that have a free license (attribution is fine; we can do that in a comment)
  • Add the SVG source code to an existing .css file (e.g. like this), so it doesn't require an extra network request (like a separate .svg file would) and also doesn't require the SVG's contents being inlined everywhere we use it.

@Anton-4
Copy link
Collaborator

Anton-4 commented Mar 23, 2024

We show a sort button to the right of the section title when the section is expanded

Sounds good!

Copy link

Thank you for your contribution! Sometimes PRs end up staying open for a long time without activity, which can make the list of open PRs get long and time-consuming to review. To keep things manageable for reviewers, this bot automatically closes PRs that haven’t had activity in 60 days. This PR hasn’t had activity in 30 days, so it will be automatically closed if there is no more activity in the next 30 days. Keep in mind that PRs marked Closed are not deleted, so no matter what, the PR will still be right here in the repo. You can always access it and reopen it anytime you like!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants