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.

Sign up for GitHub

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

Jump to bottom

LB-1722: Part B, making the link listens page more discoverable. #3151

Merged
merged 16 commits into from
Jan 28, 2025
Merged

LB-1722: Part B, making the link listens page more discoverable. #3151

merged 16 commits into from
Jan 28, 2025

Conversation

granth23
Copy link
Contributor

Problem

The page link-listens is tough to locate for new users.

Solution

image

Added a button in the link-listen popup to make the page more easy to locate.

Action

  • Let me know if any additional details or corrections are required.
  • If the changes look good, kindly proceed with merging the PR.

anshg1214
anshg1214 requested changes Jan 27, 2025
View reviewed changes
Copy link
Member

@anshg1214 anshg1214 left a comment

Choose a reason for hiding this comment

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

The PR looks good. Just some minor refactoring is required, and we should be good to go.

frontend/js/src/common/listens/MBIDMappingModal.tsx Outdated Show resolved Hide resolved
frontend/js/src/common/listens/MBIDMappingModal.tsx Outdated Show resolved Hide resolved
@granth23 granth23 requested a review from anshg1214 January 28, 2025 05:28
anshg1214
anshg1214 requested changes Jan 28, 2025
View reviewed changes
frontend/css/theme/bootstrap/modals.less Outdated Show resolved Hide resolved
frontend/js/src/common/listens/MBIDMappingModal.tsx Outdated Show resolved Hide resolved
@granth23 granth23 requested a review from anshg1214 January 28, 2025 15:19
MonkeyDo
MonkeyDo requested changes Jan 28, 2025
View reviewed changes
Copy link
Member

@MonkeyDo MonkeyDo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice work!

I've made a bunch of comments below, mainly small things related to how we usually do things in this codebase, but all minor changes and nitpicks.

Looks good!
I think along with your other PR this resolves the issue entirely 👍

frontend/js/src/common/listens/MBIDMappingModal.tsx Outdated Show resolved Hide resolved
frontend/js/src/common/listens/MBIDMappingModal.tsx Outdated Show resolved Hide resolved
frontend/js/src/common/listens/MBIDMappingModal.tsx Outdated Show resolved Hide resolved
frontend/js/src/common/listens/MBIDMappingModal.tsx Outdated Show resolved Hide resolved
frontend/js/src/common/listens/MBIDMappingModal.tsx Outdated Show resolved Hide resolved
frontend/css/main.less Outdated Show resolved Hide resolved
frontend/js/src/common/listens/MBIDMappingModal.tsx Outdated Show resolved Hide resolved
frontend/js/src/common/listens/MBIDMappingModal.tsx Outdated Show resolved Hide resolved
@granth23 granth23 requested a review from MonkeyDo January 28, 2025 16:13
MonkeyDo
MonkeyDo requested changes Jan 28, 2025
View reviewed changes
Copy link
Member

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

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

Final little forgotten change, apart from that it's good to go for me!

frontend/js/src/common/listens/MBIDMappingModal.tsx Outdated Show resolved Hide resolved
MonkeyDo
MonkeyDo approved these changes Jan 28, 2025
View reviewed changes
Copy link
Member

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

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

Good to go as far as I'm concerned!

@anshg1214 anshg1214 merged commit 8a85273 into metabrainz:master Jan 28, 2025
2 checks passed
@granth23 granth23 deleted the LB-1722--Part-B-Making-page-more-discoverable branch January 29, 2025 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anshg1214 anshg1214 anshg1214 approved these changes

@MonkeyDo MonkeyDo MonkeyDo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@granth23 @MonkeyDo @anshg1214