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

feat: Add icon toggle button #644

Merged
merged 32 commits into from
Aug 30, 2023
Merged

feat: Add icon toggle button #644

merged 32 commits into from
Aug 30, 2023

Conversation

kazaky
Copy link
Contributor

@kazaky kazaky commented Aug 22, 2023 •
edited
Loading

📋 Changes description

🤔 Context

Adding a icon toggle component to Spark

✅ Checklist

  • Link to GitHub issues it solves.
  • I have reviewed the submitted code.
  • I have tested on a phone device/emulator.
  • If it includes design changes, please ask for a review spark-design GitHub team.

Contributing has more information and tips for a great pull request.

soulcramer
soulcramer reviewed Aug 23, 2023
View reviewed changes
Copy link
Contributor

@soulcramer soulcramer left a comment

Choose a reason for hiding this comment

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

The javadoc is missing on public components otherwise LGTM

spark/src/main/kotlin/com/adevinta/spark/components/icontogglebuttons/IconToggleButtons.md Outdated Show resolved Hide resolved
spark/src/main/kotlin/com/adevinta/spark/components/icontogglebuttons/IconToggleButtons.md Outdated Show resolved Hide resolved
spark/src/main/kotlin/com/adevinta/spark/components/icontogglebuttons/IconToggleButtons.md Outdated Show resolved Hide resolved
spark/src/main/kotlin/com/adevinta/spark/components/icontogglebuttons/IconToggleButtons.md Outdated Show resolved Hide resolved
spark/src/main/kotlin/com/adevinta/spark/components/icontogglebuttons/IconToggleButtons.md Outdated Show resolved Hide resolved
spark/src/main/kotlin/com/adevinta/spark/components/icontogglebuttons/IconToggleButtons.md Outdated Show resolved Hide resolved
MarinaRomanova
MarinaRomanova reviewed Aug 23, 2023
View reviewed changes
Copy link
Contributor

@MarinaRomanova MarinaRomanova left a comment

Choose a reason for hiding this comment

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

We are missing un deprication message for the old IconToggleButton in icons package.

Ahmed Elshahawy and others added 10 commits August 23, 2023 16:34
Merge branch 'main' into icon_toggle_button
de05c88
Add icon toggle button configurator
1f7aad7
@kazaky @soulcramer
Update spark/src/main/kotlin/com/adevinta/spark/components/icontoggle…
c045f47 
…buttons/IconToggleButtons.md

Co-authored-by: Scott Rayapoullé <[email protected]>
@kazaky @soulcramer
Update spark/src/main/kotlin/com/adevinta/spark/components/icontoggle…
972ff37 
…buttons/IconToggleButtons.md

Co-authored-by: Scott Rayapoullé <[email protected]>
@kazaky @soulcramer
Update spark/src/main/kotlin/com/adevinta/spark/components/icontoggle…
226637f 
…buttons/IconToggleButtons.md

Co-authored-by: Scott Rayapoullé <[email protected]>
@kazaky @soulcramer
Update spark/src/main/kotlin/com/adevinta/spark/components/icontoggle…
2067712 
…buttons/IconToggleButtons.md

Co-authored-by: Scott Rayapoullé <[email protected]>
@kazaky @soulcramer
Update spark/src/main/kotlin/com/adevinta/spark/components/icontoggle…
fdc9183 
…buttons/IconToggleButtons.md

Co-authored-by: Scott Rayapoullé <[email protected]>
@kazaky @soulcramer
Update spark/src/main/kotlin/com/adevinta/spark/components/icontoggle…
6fa1e1a 
…buttons/IconToggleButtons.md

Co-authored-by: Scott Rayapoullé <[email protected]>
Add @deprecated to old IconToggleButton
b154301
Merge remote-tracking branch 'origin/icon_toggle_button' into icon_to…
d7bc35d 
…ggle_button
MarinaRomanova
MarinaRomanova requested changes Aug 24, 2023
View reviewed changes
spark/src/main/kotlin/com/adevinta/spark/components/icontogglebuttons/IconToggleButton.kt Outdated Show resolved Hide resolved
.../src/main/kotlin/com/adevinta/spark/components/icontogglebuttons/IconToggleButtonContrast.kt Outdated Show resolved Hide resolved
spark/src/main/kotlin/com/adevinta/spark/components/icontogglebuttons/IconToggleButtonFilled.kt Outdated Show resolved Hide resolved
spark/src/main/kotlin/com/adevinta/spark/components/icontogglebuttons/IconToggleButtonGhost.kt Outdated Show resolved Hide resolved
spark/src/main/kotlin/com/adevinta/spark/components/icontogglebuttons/IconToggleButtonIcons.kt Outdated Show resolved Hide resolved
spark/src/main/kotlin/com/adevinta/spark/components/icontogglebuttons/IconToggleButtonFilled.kt Outdated Show resolved Hide resolved
spark/src/main/kotlin/com/adevinta/spark/components/icontogglebuttons/IconToggleButtonGhost.kt Outdated Show resolved Hide resolved
.../src/main/kotlin/com/adevinta/spark/components/icontogglebuttons/IconToggleButtonOutlined.kt Outdated Show resolved Hide resolved
spark/src/main/kotlin/com/adevinta/spark/components/icontogglebuttons/IconToggleButtonTinted.kt Outdated Show resolved Hide resolved
catalog/src/main/kotlin/com/adevinta/spark/catalog/model/Components.kt Outdated Show resolved Hide resolved
kazaky and others added 8 commits August 25, 2023 09:14
@kazaky kazaky marked this pull request as ready for review August 25, 2023 07:46
@kazaky kazaky requested review from SimonMarquis and a team as code owners August 25, 2023 07:46
@kazaky kazaky requested a review from soulcramer August 25, 2023 08:36
MarinaRomanova
MarinaRomanova reviewed Aug 25, 2023
View reviewed changes
Copy link
Contributor

@MarinaRomanova MarinaRomanova left a comment

Choose a reason for hiding this comment

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

LGTM, I let @soulcramer to have a look as well

@soulcramer soulcramer merged commit e6c6175 into main Aug 30, 2023
5 checks passed
@soulcramer soulcramer deleted the icon_toggle_button branch August 30, 2023 12:57
This was linked to issues Aug 30, 2023
[IconButton] Add configurable example for Toggleable IconButton in Catalog App #634
Closed
[Component] IconButton - Add Toggleable icon button for all styles #609
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SimonMarquis SimonMarquis Awaiting requested review from SimonMarquis SimonMarquis is a code owner

@soulcramer soulcramer Awaiting requested review from soulcramer soulcramer is a code owner

@MarinaRomanova MarinaRomanova Awaiting requested review from MarinaRomanova

Assignees

@kazaky kazaky

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@kazaky @soulcramer @MarinaRomanova