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
Use LinkButton
in Excerpt
#3268
Conversation
it('displays inline controls if collapsed', () => { | ||
const wrapper = createExcerpt({ inlineControls: true }, TALL_DIV); | ||
assert.isTrue(wrapper.exists('.Excerpt__inline-controls')); | ||
assert.isTrue(wrapper.exists('InlineControls')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updating to not use a classname selector while I'm in here...
Codecov Report
@@ Coverage Diff @@
## master #3268 +/- ##
=======================================
Coverage 97.93% 97.93%
=======================================
Files 211 211
Lines 7693 7693
Branches 1756 1756
=======================================
Hits 7534 7534
Misses 159 159
Continue to review full report at Codecov.
|
f118908
to
f32bcc7
Compare
.Excerpt__toggle-link { | ||
// A container for a button with a gradient background; this gives partial | ||
// opacity behind the More/Less button so that it is easier to read | ||
.Excerpt__toggle-container { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed this classname for clarity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I see clear pros and a possible con to the visual change but I think this is the right choice. Pros: Its more obvious that it is a clickable thing, and more consistent with other link-like buttons. Con: Since it has more visual weight than before, perhaps it might distract from the body? Could also be just a matter of getting used to it.
f32bcc7
to
5a75740
Compare
This PR updates the
Excerpt
component to use aLinkButton
with a common pattern instead of a custom-styled<button>
element for its inline controls for expanding or collapsing long excerpts. There is a user-visible change here: to better match our other link-styled buttons, this button now has an underline and is not italic.Before:
After: