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

Quiet button (private component) update #372

Closed
wants to merge 97 commits into from
Closed

Quiet button (private component) update #372

wants to merge 97 commits into from

Conversation

janeszelag
Copy link

Description

Update to quietButton (private component).

Addresses issue #362 .

The changes needed (as outlined in the opened issue) were as follows:

1. Change px units to rem units: done.
2. Recommend Box instead of hard-coded padding: done.

  • With regards to the vertical padding, the TDS component Box was not able to achieve this design as there is variable spacing on top and bottom of text (ie the text is not centre-aligned). This is mentioned in the designs and as such a styled Box component was used. See the Invision design.

3. Use core-colours instead of hard-coded colours: done
4. Consider not using cursor: pointer: not done since as mentioned in the issue, a holistic effort across all buttons would be required.
5. Can leverage core-button prop types for A11yContent: done

@janeszelag janeszelag linked an issue Jul 29, 2020 that may be closed by this pull request
QuietButton follow-ups (private component) #362
Open
@TDSBot
Copy link

TDSBot commented Jul 30, 2020 

Packages pending updates:

- @tds/community-quiet-button: 0.1.1 => 0.1.2

If this is not what you expected, ensure that your commit messages follow the TDS commit types guide on this page: https://tds.telus.com/contributing/developer-guide.html and try again.

@janeszelag
Copy link
Author

I couldn't get around not updating the contributors....as a result of the script "contributors:update"

@janeszelag
Copy link
Author

tds box versioning was giving me problems with the snapshot :(

theetrain
theetrain approved these changes Jul 31, 2020
View reviewed changes
Copy link
Contributor

@theetrain theetrain left a comment

Choose a reason for hiding this comment

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

These changes look good to me! I have one optional suggestion for you below.

packages/QuietButton/QuietButton.jsx Outdated Show resolved Hide resolved
@TDSBot
Copy link

TDSBot commented Jul 31, 2020 

Packages pending updates:

- @tds/community-quiet-button: 0.1.1 => 0.1.2

If this is not what you expected, ensure that your commit messages follow the TDS commit types guide on this page: https://tds.telus.com/contributing/developer-guide.html and try again.

@theetrain
Copy link
Contributor

theetrain commented Jul 31, 2020 •
edited
Loading

Bonus request:

  • please use a minHeight for the Box in order to allow wrapping copy to have the border surround
  • ensure the icon, if used, lines up with the first word in case word wrapping occurs

image

@janeszelag
Copy link
Author

Allows for wrapping, icon lines up with top row

Screen Shot 2020-07-31 at 11 20 16 AM

@TDSBot
Copy link

TDSBot commented Jul 31, 2020 

Packages pending updates:

- @tds/community-quiet-button: 0.1.1 => 0.1.2

If this is not what you expected, ensure that your commit messages follow the TDS commit types guide on this page: https://tds.telus.com/contributing/developer-guide.html and try again.

sudhirraivns0103 and others added 28 commits September 10, 2020 12:33
@sudhirraivns0103 @mike-bunce
feat(community-tabs): changing to tabs heading
e37d8af
chore(publish): update packages
93f70f2 
 - @tds/[email protected]
@mike-bunce
fix(community-tabs): removing unnecessary dep
e94e7c8
@mike-bunce
fix(community-tabs): updating deps
20a362a
@mike-bunce
fix(community-tabs): updating deps
51b2f11
@mike-bunce
test(community-mini-expand-collapse): updating snaps
e624104
@mike-bunce
test(community-mini-expand-collapse): restoring snapshot
070bcf0
@mike-bunce
fix(community-tabs): updating deps
e8ead25
@mike-bunce
test(community-mini-expand-collapse): updating snap
ae89964
@mike-bunce
test(community-mini-expand-collapse): restoring snap
7c7ef3f
@mike-bunce
fix(community-mini-expand-collapse): adding missing prop
60d6ded
chore(publish): update packages
8348c23 
 - @tds/[email protected]
 - @tds/[email protected]
@jraff
revert(publish): update packages
6760c9e 
This reverts commit 8348c23.
chore(publish): update packages
ad716da 
 - @tds/[email protected]
 - @tds/[email protected]
@jraff
chore(deps): update lockfile
ad3b2b0
@jraff
chore(community-tabs): update dependencies
e46b450
@jraff
test(community-tabs): update snapshots
49eecbd
chore(publish): update packages
615346f 
 - @tds/[email protected]
fix(community-quiet-button): addresses issue 362, followup
c182568
fix(community-quiet-button): updates snapshot
b578548
fix(community-quiet-button): updates snapshot
4463741
fix(community-quiet-button): updates snapshot
48c95d7
fix(community-quiet-button): updates snapshot
b94eb85
fix: updates prop types
8ad8eb9
fix(community-quiet-button): uses minHeight to allow for wrapping
e8de4fd
fix(community-quiet-button): uses minHeight to allow for wrapping
50c054f
chore(community-tabs): update snap shot
5f741d3
chore: merge conflict
72bf91d
@janeszelag janeszelag closed this Oct 26, 2020
@janeszelag janeszelag deleted the quietButtonUpdate branch October 26, 2020 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

QuietButton follow-ups (private component)
7 participants
@janeszelag @TDSBot @theetrain @ruchijn @jraff @sudhirraivns0103 @mike-bunce