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

Fixes docs build #5474

Closed
wants to merge 1 commit into from
Closed

Fixes docs build #5474

wants to merge 1 commit into from

Conversation

milanholemans
Copy link
Contributor

@milanholemans milanholemans commented Sep 8, 2023 •
edited
Loading

In this PR:

  • Removed unwanted build files from the docs project. This caused the docs not to display our new changes. E.g. the Twitter icon on our team page is gone.
  • Disabled Mendable popup because it's quite annoying on desktop. Decreased the popup time on mobile from 10s to 3s.
  • Fixed option description for pp solution publisher commands.

@Adam-it Adam-it self-assigned this Sep 19, 2023
Adam-it
Adam-it approved these changes Sep 19, 2023
View reviewed changes
Copy link
Member

@Adam-it Adam-it left a comment

Choose a reason for hiding this comment

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

cool 😎
checked locally ✅
TBH I never noticed we are missing the twitter icon on our team page 😅.

Adam-it
Adam-it reviewed Sep 19, 2023
View reviewed changes
docs/src/theme/Footer/index.tsx
@@ -60,6 +60,8 @@ function Footer(): JSX.Element | null {
backgroundColor: '#ef5552'
}}
cmdShortcutKey='m'
showPopup={false}
Copy link
Member

Choose a reason for hiding this comment

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

@milanholemans just to confirm. I am not actually sure the impact of this change. Could you add a comment to this one
I only noticed that after your changes the M icon is
image

currently, on our docs page it is
image

I don't see any other bigger changes 🤔 At least on desktop 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It also disables the pop-up message from the button which is quite annoying on desktop according to me. On mobile devices the pop-up is still visible with the round button variant.

Copy link
Member

Choose a reason for hiding this comment

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

ok thanks for confirming.
Now I actually understand what you mean
image

Indeed it is annoying 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the circle icon, but it seems to be impossible to use it on desktop without the annoying popup 🤷‍♂️

@Adam-it
Copy link
Member

Adam-it commented Sep 20, 2023

ready to merge 🚀

@Adam-it
Copy link
Member

Adam-it commented Sep 21, 2023

merged manually. You Rock 🤩

@Adam-it Adam-it closed this Sep 21, 2023
@milanholemans milanholemans deleted the fix-docs branch September 21, 2023 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Adam-it Adam-it Adam-it approved these changes

Assignees

@Adam-it Adam-it

Labels
docs pr-bugfix pr-merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@milanholemans @Adam-it