Welcome back to another Repo Recap, where we cover last week's contributions to dev.to's repository and the iOS repo. This edition is covering February 17 to February 23.
Features
-
@deadlybyte added support for
<kbd>tag in posts and comments, so now you can do stuff like ctrl! Thanks, @deadlybyte!
Added <kbd> tag support to markdown editors in article posts and comm…
#1761
…ents.
What type of PR is this? (check all applicable)
- [ ] Refactor
- [x] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Added tag support to markdown editors in article posts and comments.
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
[optional] What gif best describes this PR or how it makes you feel?
Bug Fixes / Other Contributions
-
@rhymes increased the contrast for the reading time duration (ex.
1 min read). The PR has thorough information about how to do it, too. Thanks, @rhymes!
Increase contrast for "reading time" info
#1689
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
While perusing the Lighthouse report for dev.to I've noticed a "quick win" regarding the contrast between the white background and the text of the "reading time" info.
Right now the background is #fff and the reading time text is gray which is rgb(128, 128, 128) which in turn results in this contrast level:
I've darkened the text to rgb(89, 89, 89) with
color: darken($medium-gray, 5%);
instead of the current
color: lighten($medium-gray, 10%);
The contrast level now is:
I have used this color contrast analyzer linked by Lighthouse.
ps. the diff includes some auto fixed spacing issues, the relevant line his here: https://github.com/thepracticaldev/dev.to/pull/1689/files#diff-8604f2aa24d368dba9c73900d331fbbbR575
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Before
After
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
-
@lightalloy disabled
erblintcops for the editor guide page specifically to prevent a strange interaction. Thanks, Anna!
Disable erblint cops for the editor guide
#1830
- @aspittel add some extra sanitization when linking to GitHub repos in Connect. Thanks, Ali!
add sanitization to github on chat
#1834
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
Add sanitization to marked (already done on backend)
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [X] no documentation needed
- @markfilus fixed a typo on the FAQ page. Thanks for your first contribution, Mark! 🎉
Fix typo on faq page
#1836
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
Fixed typo
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- @molly_struve added a post's ID to our cache keys for in the bottom section of suggested posts. Thanks for your first contribution, Molly!
Include Article ID in Bottom Content Cache Key
#1773
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
If the additional bottom content on the Article show page is being stored under a cache key that is created only using an article's tags, then one could end up seeing a suggested article on the actual article page itself if the tags were the same as a previous article that was viewed. This does mean the cache will be reused less since it is article specific which will affect performance. I am not sure how often this cache is hit across different articles, but that is something to consider.
I also added the article ID to the list of :not_ids for grabbing the @classic_article
I did read the contributing guidelines about adding specs around bug fixes, but since none of the cacheing I saw was tested in the specs and because I could not come up with a good clean test I choose to forgo it. If you determine tests are needed let me know and I can close the PR and reopen after they are added.
Related Tickets & Documents
https://github.com/thepracticaldev/dev.to/issues/1697
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
N/A
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
What gif best describes this PR or how it makes you feel?
Something something, first one is always the worst no matter how many times you have contributed to other repos....
- @arnellebalane fixed an issue where in mobile, it was possible to scroll too far down in the sidebars. Thanks, Arnelle!
Fix mobile sidebar scrolling issue
#1787
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
When the sidebar is opened in mobile layout, the darkened background (.sidebar-bg) is longer than the actual sidebar element itself, making it possible to scroll past the bottom of the actual sidebar.
The cause of the issue is the height: 1000%; declaration set in the .sidebar-bg element. This PR refactors the sidebar behavior to not rely on the ensures that the sidebar wrapper and .sidebar-bg element as the darkened background. The darkened background is moved to the .side-bar element as box-shadow, and the .sidebar-bg elements are removed, ensuring that scrolling will not go past the bottom of the sidebar..sidebar-bg elements are sized correctly, and that the sidebar does not overflow the wrapper. With the sidebar always within the wrapper, the darkened background will not scroll as well.
Related Tickets & Documents
https://github.com/thepracticaldev/dev.to/issues/1746
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
UI stays the same after the changes are applied.
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
-
@arnellebalane also fixed some linting issues in the
views/additional_content_boxesfolder. Thanks again!
Fix linting issues in views/additional_content_boxes
#1843
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Fix linting issues in the views located in app/views/additional_content_boxes.
🐈 bin/bundle exec erblint app/views/additional_content_boxes
warning: parser/current is loading parser/ruby26, which recognizes
warning: 2.6.0-compliant syntax, but you are running 2.6.1.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
Linting 4 files with 12 linters...
.erblint-rubocop20190222-7517-2zlp5: RSpec/FilePath has the wrong namespace - should be Rails
Warning: unrecognized cop RSpec/MultipleExpectations found in .erblint-rubocop20190222-7517-2zlp5
No errors were found in ERB files
Additional question:
Some views, such as _article_content_area.html.erb has the entire file indented by several levels. erblint doesn't complain about this indentation, so I'm wondering if this is intentional and should be left like this, or should the indentation be fixed?
Related Tickets & Documents
#1842
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- @maestromac added more test coverage. ✅ Thanks, Mac!
Add more test coverage
#1829
What type of PR is this? (check all applicable)
- [x] Refactor
- [x] misc
Description
Up our test coverage a bit
Related Tickets & Documents
n/a
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
n/a
Added to documentation?
- [x] no documentation needed
New Issues and Discussions
Keyboard shortcuts for markdown editor
#1826
Is your feature request related to a problem? Please describe. Having keyboard shortcuts always comes in handy. Next to every single markdown editor (including the one I'm using to write this issue) offers at least a couple of very basic shortcuts for bold/italic/code blocks, etc. Being able to keyboard-ninja comments and posts can greatly benefit user experience.
Describe the solution you'd like Pressing Ctrl+[key] will format the currently selected text with appropriate tags:
B: **bold text**I: _italic_-
L: [link]()with a cursor moving to the position where you're one ctrl+v away from creating a valid link -
1-6: # level 1-6 headers(similar to, let's say, editor.md - Something else for liquid tags, maybe with common presets like
{% user name %}or{% github thepracticaldev/dev.to %}
The above is subject to discuss of course, there are many different implementations of markdown shortcuts that differ from one library to another.
Describe alternatives you've considered Coming back to my previously written text and manually arrowing my cursor is the only alternative (not including browser extentions, but a very quick Google search didn't bring up anything worthy).
Additional context I'm up for taking care about this if it gets approved.
- @ben raised a question "Could view linting be suppressing contributions?" A great discussion and awesome open source effort came out of it. Thanks, Ben! More next week!
Could view linting be suppressing contributions?
#1828
I have the feeling that view linting could be presenting a pretty strong burden to contributions. You could make a small contribution and get hung up with a huge linting fix chore as it currently stands.
I just feel like as an outsider without context it could add a bit too much burden at the time being. Thoughts on taking it away as a blocker to pushing and leave it as something folks can voluntarily make progress on until the app is at a closer place to being fully linted on the view with our preferred styles?
-
@lightalloy raised an issue where
CacheBuster#bust_commentmay fail when banishing user. Thanks, Anna!
CacheBuster#bust_comment may fail on banishing user
#1831
Describe the bug
Commentable is passed and referenced in the CacheBuster#bust_comment(https://github.com/thepracticaldev/dev.to/blob/master/app/labor/cache_buster.rb#L15) method, but sometimes commentable could be nil, because the comments are not destroyed on commentable (e.g. article) destroy.
To Reproduce
- Destroy an article with comments
- Try to banish a user, who left of the comments on the destroyed article
- get
undefined method 'id' for nil:NilClasserror
Expected behavior (assuming that we need to keep comments on the commentable destroy) Don't try to bust cache of the nonexistent commentable/article. At least add failing spec for the method and fix the code: check if commentable exists before referencing it.
- @jess requested a feature where we send an email when posts fetched via RSS are ready to be published. Thanks, Jess! Chime in here:
Send email when RSS posts are ready to be published
#1832
Is your feature request related to a problem? Please describe. There are lots of unpublished 'draft' posts that get pulled in from RSS feeds. I think it would be helpful if we reminded people to publish these posts!
Describe the solution you'd like We regularly check whether a user has new RSS posts and send and email if they do.
Describe alternatives you've considered We send an email each time RSS feed pulls in new articles. I'm not sure how that works exactly right now but I know we'd want to avoid sending the same email multiple times a day if we pull in 10 articles at once.
- @wuz requested a feature where we sort published posts in the dashboard by publish date. Thanks, Conlin!
Sort Dashboard by Publish date where available
#1837
Is your feature request related to a problem? Please describe. I frequently save article ideas as drafts to my dashboard to come back and work on them. For the most part this works great, but the posts on the dashboard are sorted by creation date (I assume). That leads to a situation where some posts that are published later than others are listed before them on my dashboard. Here is an example:
As you can see the dates go: "Jan 19, Jan 18, Jan 22, Jan 15". I look at my dashboard to make sure I am publishing regularly, but it is a bit difficult to tell with this sorting.
Describe the solution you'd like Add the ability to sort the dashboard by post publish date, with drafts using their created at date.
Describe alternatives you've considered I could store drafts elsewhere and have considered that, but I love writing in dev.to and like having ideas saved as drafts.
- @lightalloy opened an issue about fixing the linting issues in our views, as a part of Ben's earlier issue. Thanks, Anna!
Fix linting issues in the views
#1842
A couple of weeks ago erblint and the corresponding lint-staged hook were added to the project.
But for now there is a huge linting debt in the *.html.erb files. You can learn more about how it could affect the project and the contributions to it from the discussion #1828 .
Volunteers could help the project to eliminate the debt or make it smaller.
You can get the list of the linting issues running:
bundle exec erblint --lint-all
Get the list for of issues for the specific file or directory:
bundle exec erblint app/views/%path_to_file_or_directory%
You can use --autocorrect flag, but be careful with it, cause it sometimes fixes indentation incorrectly.
I would recommend running
bundle exec erblint --autocorrect %path_to_file%` and check manually if the fixes are correct.
Please, keep the pull requests small, so they could be reviewed faster. E.g. a separate pull request for each of the app/views/ folder (e.g. app/views/comments).
If there're some issues you can't fix, please, leave a comment to this issue, or commit with a --no-verify flag and mention it in your pull request.
I would appreciate your help with this issue.
-
@kayis raised a bug where saving a post with a different canonical URL raises an error
canonical_url has already been taken.:thinking_face: Thanks, @kayis!
Canonical url has already been taken
#1845
Describe the bug
I wanted to save an article with a canonical_url, but got an error.
To Reproduce Steps to reproduce the behavior:
- Write an article
- Save it
- Set the
canonical_urlto an URL that indeed is already taken - Save it
- Get the error "Canonical url has already been taken"
- Go back and change the
canonical_urlto an URL that isn't taken - Save it
- Still get the error "Canonical url has already been taken"
Expected behavior It should just save my article with the new canonical URL.
Desktop (please complete the following information):
- OS: Windows 7
- Browser: Firefox
- Version: 66
- @wes requested a feature where you can more easily add photo attribution to a cover image. Thanks, @wes!
Allow photo credit attribution for cover_image
#1849
It is common practice to add credits to pictures used in articles. I'd propose adding more properties to FrontMatter so that a <figcaption> element is added below the photo.
Describe the solution you'd like
cover_image: https://cdn-images-1.medium.com/max/1024/1*NQRv_y3HIXXvCOt5UWLjzQ.jpeg
cover_image_credit_text: Photo by John Smith
cover_image_credit_url: https://example.org/
Describe alternatives you've considered I'm using the end of my post for the attribution, but that's not ideal IMO.
Additional context
- @thejaredwilcurt raised a bug where numbered lists are broken out of order if a code block is inserted in between the list. Thanks, Jared!
Markdown: Code blocks in numbered lists
#1855
On GitHub, Reddit, etc. I can do this
- The first step is to
npm install - Next add this code:
console.log('some code');
- Step 3 is to...
- The the fourth step is....
- Step 5...
But on dev.to it looks like this:
- The first step is to
npm install - Next add this code:
console.log('some code');
- Step 3 is to...
- The the fourth step is....
- Step 5...
If there is a way to embed code blocks into ordered lists without breaking the ordering, it is not documented anywhere.
This is also a pattern that is slightly different on many different sites, where the number of spaces required at the start of the code block varies, whether it allows syntax highlighting in list code blocks, whether it allows triple graves, and whether or not the remaining lines need indentation.
There are a lot of possible permutations of how this could be implemented and without a live mardown preview, doing trial-and-error to attempt to make it work is frustrating. You need to add or remove a space at the start of every line of code, click preview, scroll scroll scroll down, and then click preview again to go back and try something else.
- @defman opened the discussion of whether we should have a feature that allows us to ignore tags you don't want to see on the feed. Thanks, @defman!
Ignore/blacklist tags
#1856
Is your feature request related to a problem? Please describe. The problem is that I want some posts to disappear from my feed.
Describe the solution you'd like I want to have the ability to ignore a tag.
Describe alternatives you've considered Looks like the only way to achieve it atm is following the tag and setting its weight to -999 or something. I don't want to follow tags that I want to ignore.
Additional context Something like that maybe?
DEV-iOS
We haven't had any new issues or PRs merged lately. Feel free to check out the iOS repo, or download our iOS app on the App Store.
That's it for this week. Thanks for reading!
Top comments (1)
Thank you for your contributions @deadlybyte , @ben , @rhymes , @lightalloy , @aspittel , @markfilus , @molly_struve , @arnellebalane , @maestromac , @mkrl , @jess , @wuz , @kayis , @wes , @thejaredwilcurt , and @defman !