-
Notifications
You must be signed in to change notification settings - Fork 3
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
Dashboard: promote features on sidebar #333
Conversation
Initial iteration to promote some features to our users. Closes #103
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great start. I feel these all might benefit from a link/button call to action to though, as the only thing linking these is the header right now and the headers are goal focused (Preview changes before building!
) instead of actionable (Enable pull request previews
)
readthedocsext/theme/templates/includes/elements/promotions/example-projects.html
Outdated
Show resolved
Hide resolved
Writing documentation is hard. | ||
We know it. | ||
Get started quickly with these example projects for a few common tools. | ||
With our example projects, you can focus on writing documentation, which is what you love, right? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like it's making two points. Both are valid, but if feels like the better focus is pointing the user how to get started instead.
Example projects guide the user on how to bootstrap a project, users don't start off blocked on writing documentation without having a project yet. This should probably focus on "Not sure how to get started?" instead. We're mostly concerned about the user learning how to use Read the Docs with this content.
The tutorial is probably an even better resource, or a good resource in addition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated it a little bit. Let me know if that's fine or you prefer to change it completely and show the tutorial. In any case, feel free to make a suggestion.
readthedocsext/theme/templates/includes/elements/promotions/pull-request-previews.html
Outdated
Show resolved
Hide resolved
readthedocsext/theme/templates/includes/elements/promotions/pull-request-previews.html
Outdated
Show resolved
Hide resolved
readthedocsext/theme/templates/includes/elements/promotions/ruby-support.html
Outdated
Show resolved
Hide resolved
readthedocsext/theme/templates/includes/elements/promotions/ruby-support.html
Outdated
Show resolved
Hide resolved
readthedocsext/theme/templates/includes/elements/promotions/security-logs.html
Outdated
Show resolved
Hide resolved
Security logs allow you to <em>audit activity</em> around logins and team changes within your organization. | ||
We store the IP address and the browser on each event, so that you can confirm this access was from the right person. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This block is the default if all other blocks are not shown, so we could potentially be showing this to community users. It seems we should focus this not just on organization security logs. Perhaps this is just the wrong fall back content?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. We need to think about a better fallback content for community 👍🏼
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can point to the addons blog post once it's merged/published: readthedocs/website#278
Co-authored-by: Anthony <aj@ohess.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks great! I noted some simplification and minor style changes on the card structure, but what you have there is great
<div class="ui grid extra content"> | ||
<a class="ui sixteen wide column positive basic button" target="_blank" href="{{ url }}"> | ||
{% trans "Check out example projects" %} | ||
</a> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These don't need grids to affect the content width of the button and can instead use a fluid
button. I would use primary
instead of positive
too. The card examples show positive
/negative
as an example of a yes/no dialog prompt, where instead we just want a nice looking link.
And the button looks quite big, I might try small
to reduce it just a little.
<div class="ui grid extra content"> | |
<a class="ui sixteen wide column positive basic button" target="_blank" href="{{ url }}"> | |
{% trans "Check out example projects" %} | |
</a> | |
</div> | |
<div class="ui extra content"> | |
<a class="ui small primary fluid basic button" target="_blank" href="{{ url }}"> | |
{% trans "Check out example projects" %} | |
</a> | |
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is how it looks with this change:
I think it's fine 👍🏼
These don't need grids to affect the content width of the button and can instead use a
fluid
button
Honestly, I'm not able to understand how this pattern works yet and I had pretty bad time trying out these different classes 😅 until I found something that didn't break. I need to go back to the basics of this framework at some point and try to understand it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes you have here look like what I was describing, yeah. But the code in the PR is still not updated it seems.
readthedocsext/theme/templates/projects/partials/dashboard/traffic-analytics.html
Outdated
Show resolved
Hide resolved
I think this is ready to merge. @agjohnson can you make a final review here? |
Did you forget to push up? I don't see any changes since last review. |
Meh, yeah, I pushed but it failed because I didn't have my SSH keys into the agent 👎🏼 . I just pushed the changes. |
* Announcements: allow user to close feature announcements Add a small "X" next in the right top corner of the announcement to let the users to close the announcement. It uses a cookie to avoid showing it next time they load the page. Related * #333 * #103 * Add the hide button logic to all the announcements
Initial iteration to promote some features to our users.
Closes #103
Requires readthedocs/readthedocs.org#11287