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.

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

Fixes #3644 - Add a "moved" milestone #3645

Merged
merged 1 commit into from
Nov 21, 2021
Merged

Fixes #3644 - Add a "moved" milestone #3645

merged 1 commit into from
Nov 21, 2021

Conversation

ksy36
Copy link
Contributor

@ksy36 ksy36 commented Nov 18, 2021

This PR adds a "moved" milestone for issues that were moved to other bug trackers or repositories (bugzilla, fenix, firefox-ios, etc.)

@@ -43,6 +43,7 @@ export const Issue = Backbone.Model.extend({
var milestoneClass;
if (response.milestone) {
milestone = response.milestone.title;
milestoneClass = response.milestone.title;
} else {
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've noticed that milestone labels were not coloured on the issue page, because a css class was missing, this is a fix for that.

currently:
Screen Shot 2021-11-18 at 5 53 12 PM
will be:
Screen Shot 2021-11-18 at 5 53 32 PM

@@ -15,5 +15,5 @@
<span class="label label-<%= data.milestoneClass %> js-Milestone">
<%= data.milestone %>
</span>
</span>
</div>
</script>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There seems to be closing </span> instead of a </div>

@ksy36
Copy link
Contributor Author

ksy36 commented Nov 18, 2021

r? @karlcow

@ksy36 ksy36 requested a review from karlcow November 18, 2021 22:55
Copy link
Member

@karlcow karlcow left a comment

Choose a reason for hiding this comment

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

Thanks @denschub for the issue and @ksy36 for the implementation

This is really cool.

A simple change on the order.

Also do not forget to create the actual milestones.

@@ -100,7 +100,8 @@
'invalid': {'id': 0, 'order': 4, 'state': 'closed'},
'non-compat': {'id': 0, 'order': 5, 'state': 'closed'},
'wontfix': {'id': 0, 'order': 6, 'state': 'closed'},
'worksforme': {'id': 0, 'order': 7, 'state': 'closed'}}
'worksforme': {'id': 0, 'order': 7, 'state': 'closed'},
'moved': {'id': 0, 'order': 1, 'state': 'closed'}}
Copy link
Member

Choose a reason for hiding this comment

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

see comment #3645 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good to know, I'll change that :)

@@ -43,6 +43,7 @@ export const Issue = Backbone.Model.extend({
var milestoneClass;
if (response.milestone) {
milestone = response.milestone.title;
milestoneClass = response.milestone.title;
Copy link
Member

Choose a reason for hiding this comment

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

good catch!

@@ -15,5 +15,5 @@
<span class="label label-<%= data.milestoneClass %> js-Milestone">
<%= data.milestone %>
</span>
</span>
</div>
Copy link
Member

Choose a reason for hiding this comment

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

ooops

@ksy36
Copy link
Contributor Author

ksy36 commented Nov 19, 2021

I've made the change, could you please take another look @karlcow?

Copy link
Member

@karlcow karlcow left a comment

Choose a reason for hiding this comment

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

Thanks! 🌊

@karlcow karlcow merged commit 67f2831 into main Nov 21, 2021
@ksy36 ksy36 deleted the issues/3644/1 branch November 22, 2021 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants