Skip to content
Snippets Groups Projects

Milestone tab badges

Merged username-removed-408881 requested to merge gitlab-ce-milestone-tab-badges into master

What does this MR do?

Adds count badges to the tabs on the Milestone page to mirror the Issue page.

Are there points in the code the reviewer needs to double check?

CSS: Padding was added to the .milestone element to make expired milestones look good.

Why was this MR needed?

For consistency with other tabs

What are the relevant issue numbers?

Closes #20114 (closed)

Screenshots (if relevant)

Before

After

Screen_Shot_2016-08-23_at_1.42.56_AM

Screen_Shot_2016-08-23_at_1.43.15_AM

Does this MR meet the acceptance criteria?

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
10
11 let(:counts) { helper.milestone_counts(project.milestones) }
12
13 it 'returns a hash containing three items' do
14 expect(counts.length).to eq 3
15 end
16 it 'returns a hash containing "opened" key' do
17 expect(counts.has_key?(:opened)).to eq true
18 end
19 it 'returns a hash containing "closed" key' do
20 expect(counts.has_key?(:closed)).to eq true
21 end
22 it 'returns a hash containing "all" key' do
23 expect(counts.has_key?(:all)).to eq true
24 end
25 # This throws a "NoMethodError: undefined method `+' for nil:NilClass" error for line 27; can't figure out why it can't find the keys in the hash
  • @alfredo1 I'm an idiot and reviewed https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5921, forgetting about the previous implementation of this. I think this one's the one to stick with, as it runs two fewer queries and you've been kind enough to pick it up.

    Do you know why this one needed CSS changes and the other one didn't?

  • @smcgivern here are two screenshots showing the CSS fixes

    When milestones were expired the elements did not looked good with the gray background paddings-1

    On mobile there was no space around buttons paddings-2

  • Thanks @alfredo! That makes sense.

  • Added 1 commit:

  • Added 1 commit:

    • 1f49f28f - Ensure milestone counts work with no data
  • @alfredo1 done and pushed! :ok_hand:

  • Added 47 commits:

    • 1f49f28f...55de9cc8 - 35 commits from branch master
    • ae3a2859 - Show badges in Milestone tabs and padding in Milestone list (both to match Issues page)
    • 4b2f285b - Changeling
    • 291e486d - Improved milestone counts with a single query
    • 6c5e1011 - Use newer hash style
    • 4801bfe7 - Updated milestone count helper plus tests
    • 3d9e457e - Take out of /dashboard/milestones as the helper query clashes with kaminari arrays
    • 7786fdfb - Added a small helper to reduce logic in the view
    • b9e43392 - Update CHANGELOG
    • f9ed521a - Add paddings to milestone action buttons on mobile
    • 6a2eab44 - Update CHANGELOG
    • b71f9245 - Add space to make rubocop happy
    • 2e104b98 - Fix failing specs
  • Added 1 commit:

  • Added 2 commits:

    • 44015388 - Fix failing specs and improve html
    • 66db765c - Ensure milestone counts work with no data
  • @smcgivern does the backend part look good to you? so I can assign to a frontend endboss to review the frontend part.

  • @alfredo1 sorry, yes it does! It looks like @jschatz1 is the only frontend endboss (too many 'end's!) around this week so I'll assign to him.

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading