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

Improve method source toggling #1176

Merged
merged 9 commits into from
Oct 10, 2024
Merged

Improve method source toggling #1176

merged 9 commits into from
Oct 10, 2024

Conversation

st0012
Copy link
Member

@st0012 st0012 commented Sep 8, 2024

  1. Source toggling control is now always displayed
    • If we think the buttons take too much space or are distracting, I'd be happy to accept ideas/PRs for styling improvements. But hiding them by default simply made this feature underutilized.
  2. Source will be displayed directly under the method method
    • This matches rubyapi.org's behaviour and makes a more natural experience. When viewing a method with long description, users won't need to click the toggle, and then scroll all the way down to see the source.
  3. Clicking the method name will not toggle the source anymore. I made this change so we can later make clicking method names link to the methods instead, like rubyapi.org and many other language documentations do.

Demos

Before

Screen.Recording.2024-09-08.at.23.41.03.mov

After

Screen.Recording.2024-10-03.at.13.15.09.mov

@st0012 st0012 changed the title Improve method toggling Improve method source toggling Sep 9, 2024
@st0012 st0012 marked this pull request as ready for review September 9, 2024 10:36
@etiennebarrie
Copy link
Contributor

3. Clicking the method name will not toggle the source anymore. I made this change so we can later make clicking method names link to the methods instead, like rubyapi.org and many other language documentations do.

Even if we decide to keep the source toggle on the method name, I'd love to get the URL on the method name so that at least we can right-click, copy link location and share that more easily.

@st0012
Copy link
Member Author

st0012 commented Sep 9, 2024

Does this look good to you? (ofc, I'm not going to make all method names green)

Screen.Recording.2024-09-09.at.13.18.40.mov

Copy link

@rwstauner rwstauner left a comment

Choose a reason for hiding this comment

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

These are great improvements, thank you!

@paracycle
Copy link
Contributor

I think this looks great, but I have one concern.

If you look at the following screenshot from one of your After demos, can you tell me quickly which code section is the source and which one is a part of the docs?
image

IMO, we need a distinguishing background or border or something (a designer would know better than me) for the "source code" section that opens up.

@p8
Copy link
Contributor

p8 commented Sep 9, 2024

Maybe move the toggle to the bottom, so the toggle and the source code are closer together?
SDoc currently does this:
image

@st0012
Copy link
Member Author

st0012 commented Sep 9, 2024

I think this looks great, but I have one concern.

That's a good point. I'll think about how to avoid the confusion.

Maybe move the toggle to the bottom, so the toggle and the source code are closer together?

Since it's been at the top for a long time, moving it to the bottom would require a huge change on users' behaviour. It's also not uncommon to see methods having very long description and examples in Ruby (e.g. Array#[], Array#fill...etc.), and IMO being able to click and see the source toggle without scrolling further down would be a better experience.

@st0012
Copy link
Member Author

st0012 commented Sep 12, 2024

To help distinguishing source code blocks and example code blocks, I ended up choosing to add Example label to code blocks interpreted as Ruby:

Screenshot 2024-09-12 at 13 24 37 Screenshot 2024-09-12 at 12 48 50

Notes

  • It's easier to add Source label but I feel it a bit repetitive when you just clicked Source and then see the shown block marked as Source again.
  • Styling options are limited because the html of example code blocks are NOT controlled by the Darkfish generator but directly from RDoc. So if we were to change its html structure, it could break gems that use RDoc (e.g. SDoc)

@st0012 st0012 force-pushed the improve-method-toggling branch 2 times, most recently from 8cf49cb to 7ec0a4c Compare September 12, 2024 12:14
@p8
Copy link
Contributor

p8 commented Sep 13, 2024

Styling options are limited because the html of example code blocks are NOT controlled by the Darkfish generator but directly from RDoc. So if we were to change its html structure, it could break gems that use RDoc (e.g. SDoc)

You should be able to change the background of the source with the following:

.method-source-code pre {
  background: #ebead5;
}
image

Also SDoc uses it's own css and templates, so it shouldn't be affected.
It uses rouge for code highlighting: rails/sdoc#239

st0012 added 7 commits October 3, 2024 13:07
Currently, if a method description is long (e.g. `Array.new`), users need
to click the method toggle button next to the method title, and then scroll
down to the source code expanded below the description.

This commit changes the behavior so that the source code is expanded
immediately below the method title.
1. Display the method toggle button by default instead of displaying on hover
2. Only toggle the source code when clicking the method toggle button, not
   when clicking the entire method title section. This will allow us to display
   an anchor link next to the method title
3. Simplify the toggle source button's appearance
By moving the method controls out of the method header, we can display
them to the right of the method name on desktop, and below the method name
on mobile.
The label should help users distinguish example code blocks from other
code blocks, such as method source code.

It's only applied to Ruby code examples.
@st0012 st0012 force-pushed the improve-method-toggling branch from 0630c76 to 5bbdee7 Compare October 3, 2024 12:07
@st0012
Copy link
Member Author

st0012 commented Oct 3, 2024

I decided to go with @p8's suggestion to use a different background color for the source code block (see the updated demo vid) because it introduces less complexity.

@st0012 st0012 requested a review from colby-swandale October 3, 2024 12:17
@p8
Copy link
Contributor

p8 commented Oct 3, 2024

Looks great!

@colby-swandale
Copy link
Member

I have a couple of items:

  • The various yellows on the page doesn't seem to blend well together and makes the page distracting. Could we tweak the colors a bit to make it easier on the eyes?
Screenshot 2024-10-03 at 11 45 23 PM
  • The arrow/collaspe icon next to the Source link is a bit confusing to me. It comes across as a play button for audio/video nearly everytime I look at It. Could we look to remove it and give the link an active/hover state?

@st0012
Copy link
Member Author

st0012 commented Oct 3, 2024

I made their color match and change it to fit the current color scheme better:

Screenshot 2024-10-03 at 17 25 23

I'm not happy with the color but I think further improvements on it belongs to #1164

Could we look to remove it and give the link an active/hover state?

I think this is more subject to personal taste? To me it's a clear sign that it's a details element and I can expand it. It's the default triangle that comes with the element and it's in the sidebar too.

I don't mind it being redesigned later but I don't see the value of introducing yet another UI effect to remind users it's click/expandable.

@st0012 st0012 force-pushed the improve-method-toggling branch from 5bbdee7 to fd9fbcf Compare October 3, 2024 16:59
@st0012
Copy link
Member Author

st0012 commented Oct 8, 2024

@colby-swandale Can you give this another look please?

@st0012 st0012 merged commit e608a84 into master Oct 10, 2024
51 checks passed
@st0012 st0012 deleted the improve-method-toggling branch October 10, 2024 09:45
matzbot pushed a commit to ruby/ruby that referenced this pull request Oct 10, 2024
(ruby/rdoc#1176)

* Move method source block to the top

Currently, if a method description is long (e.g. `Array.new`), users need
to click the method toggle button next to the method title, and then scroll
down to the source code expanded below the description.

This commit changes the behavior so that the source code is expanded
immediately below the method title.

* Update method toggle's interface

1. Display the method toggle button by default instead of displaying on hover
2. Only toggle the source code when clicking the method toggle button, not
   when clicking the entire method title section. This will allow us to display
   an anchor link next to the method title
3. Simplify the toggle source button's appearance

* Use details tag for method toggling

* Rename method-click-advice to method-source-toggle

* Improve method controls' display on mobile

By moving the method controls out of the method header, we can display
them to the right of the method name on desktop, and below the method name
on mobile.

* Add "Example" label to example code blocks

The label should help users distinguish example code blocks from other
code blocks, such as method source code.

It's only applied to Ruby code examples.

* Revert "Add "Example" label to example code blocks"

This reverts commit ruby/rdoc@69fc9ce6a379.

* Give source code blocks a different background color

* Change targeted method's highlighting color to work better with the new method source

ruby/rdoc@e608a84af3
@st0012 st0012 added this to the v6.8.0 milestone Oct 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

6 participants