Remove "Show More" button from tables #10891
Conversation
|
|
|
Any idea why are the unit-tests failing? What do i do to prevent it? |
|
This pull request generated screenshots of many common pages in the running app. You should be able to download and view them here: |
I wasn't sure about the 5 to 3 part, and has changed the z-index right now, as it was the only plausible change I could figure out.
|
This pull request generated screenshots of many common pages in the running app. You should be able to download and view them here: |
|
@TildaDares I have updated and now the checks are passing. Please let me know if it is good to merge now! :') |
|
Sorry for the confusion. I hope this is correct now! Edit: Ahh, alot of checks are failing now :(. How to resolve and where can I see the error log? |
app/views/grids/_notes.html.erb
Outdated
| <style> | ||
| th { | ||
| position: sticky; | ||
| top: 0; | ||
| z-index: 5; | ||
| z-index: 3; |
There was a problem hiding this comment.
@Yavnikaa We don't need this change.
There was a problem hiding this comment.
Updated that.
As per your previous comment, I figured out that I need to change the file test/unit/node_shared_test.rb , and replace 5 by 3. But I don't understand how did removing show more bring about these tests failing. Can you help me with this, and am I doing it right as per the error log in the comment?
There was a problem hiding this comment.
But I don't understand how did removing show more bring about these tests failing.
All those tests that are failing count the number of a specific class name in the code. When you removed the JavaScript code, some of those classes were also removed. That's why we have to update the number from 5 to 3.
|
This pull request generated screenshots of many common pages in the running app. You should be able to download and view them here: |
|
Hi @Yavnikaa it looks like there are just a few more places to make this change in test/unit/node_shared_test.rb -- see the errors starting on this line: https://github.com/publiclab/plots2/runs/5821145214?check_suite_focus=true#step:5:81 Do you think you can make the additional changes? then this should be all good, thank you so much! |
|
Code Climate has analyzed commit 09c8fa4 and detected 0 issues on this pull request. View more on Code Climate. |
|
This pull request generated screenshots of many common pages in the running app. You should be able to download and view them here: |
|
I think it works fine now @TildaDares and @jywarren . Can this be merged now? |
@Yavnikaa You can look for issues with the help-wanted or bug tag or create FTOs for another first-timer. |
|
Great work, thank you!! |
Fixes #10817