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

collapse block fix #5463

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

collapse block fix #5463

wants to merge 6 commits into from

Conversation

kemsguy7
Copy link

@kemsguy7 kemsguy7 commented Oct 23, 2024 •
edited by chalin
Loading

This pr resolves issue: #5405 where I resolved an issue in the nodeJs documentation Page; improving the visibility of long invisible code blocks wrapped around previous with HTML summary tag .

Key changes

  • Created a new shortcode preview-fade.html in the themes/docsy/layouts/shortcodes directory to hold the expandable Show More that shows the first 3 lines of the codeblock
  • Added in styling for the new Layout by creating a new scss file in themes/docsy/assets/scss/shortcodes/preview-fade.scss
  • Edited the Markdown file to include my newly custom-created shortcode
  • Added a Javascript file for the Show More to toggle the collapsible section based on its state in themes/docsy/assets/js/preview-fade.js

Demo

Screenshot 2024-10-23 at 03 33 56

Preview

@kemsguy7 kemsguy7 requested a review from a team as a code owner October 23, 2024 02:55
@opentelemetrybot opentelemetrybot requested a review from a team October 23, 2024 02:55
@kemsguy7
Copy link
Author

@svrnm , please kindly review.

Also I have an issue with linking the Javascript. Please where do you think I should link it, should that be in the root asset folder or in the theme(docsy) asset folder?

@kemsguy7 kemsguy7 changed the title Kemsguy7 collapse block fix [OUTREACHY]: Kemsguy7 collapse block fix Oct 23, 2024
@svrnm svrnm changed the title [OUTREACHY]: Kemsguy7 collapse block fix collapse block fix Oct 23, 2024
@svrnm
Copy link
Member

svrnm commented Oct 23, 2024

@svrnm , please kindly review.

Also I have an issue with linking the Javascript. Please where do you think I should link it, should that be in the root asset folder or in the theme(docsy) asset folder?

In the root asset folder. the theme is a git submodule and shouldn't be touched. If we like the solution we can talk about upstreaming it to docsy (cc @chalin)

Can you add the javascript and the shortcode file you used? right now the PR is broken and I can not preview it.

@kemsguy7
Copy link
Author

kemsguy7 commented Oct 23, 2024 •
edited
Loading

@svrnm here's an update on this pr

I moved all files to the root directory,

-The shortcode can be found in layouts/shortcodes/preview-fade.html
-The scss file can be found in assets/scss/_preview-fade.scss, I also imported the preview scss into the _styles_project.scss file
-The javascript can be found in assets/js/preview-fade.js,
-I also updated the layouts/partials/hooks/body-end.html with the js import
{{ partial "script.html" (dict "src" "js/preview-fade.js") -}}

The shortcode and assets are not being applied after moving them to the root directory

@svrnm
Copy link
Member

svrnm commented Oct 25, 2024

@kemsguy7 thanks!

Please take a look at the preview, it seems there is something not working as expected:

https://deploy-preview-5463--opentelemetry.netlify.app/docs/languages/js/getting-started/nodejs/

svrnm
svrnm requested changes Oct 25, 2024
View reviewed changes
content/en/docs/languages/js/getting-started/nodejs.md Outdated
Comment on lines 19 to 20
## Prerequisites
Copy link
Member

Choose a reason for hiding this comment

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

make sure you do not apply changes that are unrelated to this PR. You may be able to remove them easily by running npm run fix:format locally, otherwise take a look into the files and remove all unrelated changes.

@kemsguy7
Copy link
Author

kemsguy7 commented Oct 25, 2024 •
edited
Loading

@svrnm , i've found and fixed the issue

  • I moved the shortcode styles from assets/scss/_preview-fade.scss, Into the assets/scss/_styles_project.scss file.
  • l also made some changes to the javascript to align with the HTML shortcode. Attached is a demo video showing how it works.
collapsible.button.mp4

chalin
chalin requested changes Nov 2, 2024
View reviewed changes
Copy link
Contributor

@chalin chalin 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 this proposal @kemsguy7.

@svrnm - at first glance, my preference would be for something much simpler. Can we discuss this (possibly at the next OTel comms meeting) before moving forward with a solution?

@chalin chalin mentioned this pull request Nov 2, 2024
Open
@svrnm svrnm linked an issue Nov 6, 2024 that may be closed by this pull request
Provide a better visualization for collapsing long blocks of code/output #5405
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chalin chalin chalin requested changes

@svrnm svrnm svrnm requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
sig:javascript
Projects
SIG Comms: PRs & Issues
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Provide a better visualization for collapsing long blocks of code/output
3 participants
@kemsguy7 @svrnm @chalin