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

BB-426: fix text overflow on buttons on small screens #378

Merged
merged 6 commits into from
Mar 13, 2020
Merged

BB-426: fix text overflow on buttons on small screens #378

merged 6 commits into from
Mar 13, 2020

Conversation

endurance21
Copy link
Contributor

Problem

https://tickets.metabrainz.org/browse/BB-426?filter=-2

Solution

no more text overflow on device having width below "451px"'
solved-text-overflow

Areas of Impact

src/client/entity-editor/edition-section/edition-section.js
src/client/stylesheets/style.less

@coveralls
Copy link

coveralls commented Feb 23, 2020 •
edited
Loading

Coverage Status

Coverage increased (+0.02%) to 43.575% when pulling 2da47e0 on endurance21:uiImprovement into fe5f226 on bookbrainz:master.

@endurance21
Copy link
Contributor Author

@MonkeyDo
Is it okay that, I had used I'd css selector rather than making hierarchy of class css selector.?.

MonkeyDo
MonkeyDo requested changes Feb 28, 2020
View reviewed changes
Copy link
Contributor

@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.

I think you can find a more generic approach that would benefit other buttons where this problem could happen.

src/client/entity-editor/edition-section/edition-section.js Outdated Show resolved Hide resolved
src/client/stylesheets/style.less Outdated Show resolved Hide resolved
@endurance21
Copy link
Contributor Author

@MonkeyDo
done and dusted !
have a look!

MonkeyDo
MonkeyDo requested changes Mar 12, 2020
View reviewed changes
Copy link
Contributor

@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.

OK, after this small change it's ready to merge!
Thanks!

src/client/stylesheets/style.less Outdated
Comment on lines 415 to 416


Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove the extra blank lines here

@endurance21
Copy link
Contributor Author

@MonkeyDo
Sorry for the extra lines 😅😅, I am on the wayyyy😁

@endurance21
Copy link
Contributor Author

@MonkeyDo
done and dusted !

@MonkeyDo MonkeyDo changed the title fixed text overflow on add edition page BB-426: fix text overflow on buttons on small screens Mar 13, 2020
@MonkeyDo MonkeyDo merged commit 5483c2f into metabrainz:master Mar 13, 2020
@MonkeyDo
Copy link
Contributor

MonkeyDo commented Mar 13, 2020 •
edited
Loading

Thanks @endurance21 !

MonkeyDo
MonkeyDo reviewed Mar 27, 2020
View reviewed changes
Copy link
Contributor

@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.

This has been reported to break layout in other places like the search page:
Capture d’écran 2020-03-27 à 12 32 07

@prabalsingh24
Copy link
Contributor

@endurance21 Can I do this?

@endurance21
Copy link
Contributor Author

@prabalsingh24 ya sure.

MonkeyDo added a commit that referenced this pull request Apr 8, 2020
@MonkeyDo
fix(BB-426): Wrap text only on specific buttons
441d8b3 
A previous fix in PR #378 was causing an issue.
We only want this text wrap behavior on buttons with a lot of text, so add a class selector.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MonkeyDo MonkeyDo MonkeyDo requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@endurance21 @coveralls @MonkeyDo @prabalsingh24