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

🪟 🧹 Credits page component cleanup #21870

Merged
merged 14 commits into from
Feb 7, 2023
Merged

🪟 🧹 Credits page component cleanup #21870

merged 14 commits into from
Feb 7, 2023

Conversation

teallarson
Copy link
Contributor

@teallarson teallarson commented Jan 25, 2023 •
edited
Loading

What

Tackles some preparation for the Billing Insights PR.

  • migrates (nearly) all components on the credits page to scss modules
  • implements components from our UI library to reduce amount of custom styling
  • adds types for new endpoint data

Minimal visual changes due to using our standard spacing variables. Though one visual improvement is that the connection column now displays correctly!

Screenshot 2023-01-31 at 2 53 15 PM

How

✨ magic ✨ (jk, it's code.)

Recommended reading order

Not critical, but it may make most sense to start with the CreditsPage and work your way down the component tree.

@octavia-squidington-iv octavia-squidington-iv added the area/frontend Related to the Airbyte webapp label Jan 25, 2023
@teallarson teallarson force-pushed the teal/update-cloud-types-for-credit-consumption-api branch from 68e1430 to 18f10af Compare January 27, 2023 18:27
@teallarson teallarson force-pushed the teal/update-cloud-types-for-credit-consumption-api branch from 18f10af to 36d06cf Compare January 27, 2023 18:40
@teallarson teallarson changed the title 🪟 🧹 Clarify details on credit consumption page 🪟 🧹 migrate credit components to use scss modules and ui library components Jan 30, 2023
@teallarson teallarson changed the title 🪟 🧹 migrate credit components to use scss modules and ui library components 🪟 🧹 migrate billing page components to use scss modules and ui library components Jan 30, 2023
teallarson
teallarson commented Jan 31, 2023
View reviewed changes
airbyte-webapp/src/packages/cloud/lib/domain/cloudWorkspaces/types.ts
Comment on lines +22 to +23
connectionName: string;
status: ConnectionStatus;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New data added to this endpoint in https://github.com/airbytehq/airbyte-platform-internal/pull/4164

teallarson
teallarson commented Jan 31, 2023
View reviewed changes
airbyte-webapp/src/packages/cloud/views/credits/CreditsPage/components/CreditsUsage.module.scss

.chartWrapper {
width: 100%;
height: 260px;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we don't currently have any standardized height variables, but i wonder if we should.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think in a lot of cases it will depend on the specific component. I did recently add button heights to _variables.scss because they were referenced in e.g. table styling, so there is some precedent! https://github.com/airbytehq/airbyte/blob/master/airbyte-webapp/src/scss/_variables.scss#L41-L43

@teallarson teallarson changed the title 🪟 🧹 migrate billing page components to use scss modules and ui library components 🪟 🧹 Credits page component cleanup Jan 31, 2023
@teallarson teallarson marked this pull request as ready for review January 31, 2023 20:01
josephkmh
josephkmh approved these changes Feb 7, 2023
View reviewed changes
Copy link
Contributor

@josephkmh josephkmh left a comment

Choose a reason for hiding this comment

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

Nice! Much cleaner 🙂 just left a couple CSS ideas, but approving since they are not blocking.

airbyte-webapp/src/packages/cloud/views/credits/CreditsPage/components/CreditsUsage.module.scss

.chartWrapper {
width: 100%;
height: 260px;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think in a lot of cases it will depend on the specific component. I did recently add button heights to _variables.scss because they were referenced in e.g. table styling, so there is some precedent! https://github.com/airbytehq/airbyte/blob/master/airbyte-webapp/src/scss/_variables.scss#L41-L43

...te-webapp/src/packages/cloud/views/credits/CreditsPage/components/ConnectionCell.module.scss Outdated Show resolved Hide resolved
airbyte-webapp/src/packages/cloud/views/credits/CreditsPage/components/ConnectionCell.tsx Outdated Show resolved Hide resolved
@teallarson teallarson enabled auto-merge (squash) February 7, 2023 20:07
@teallarson teallarson merged commit 85a6026 into master Feb 7, 2023
@teallarson teallarson deleted the teal/update-cloud-types-for-credit-consumption-api branch February 7, 2023 20:32
@github-actions github-actions bot mentioned this pull request Feb 8, 2023
Closed
@octavia-squidington-iii octavia-squidington-iii mentioned this pull request Feb 8, 2023
Closed
danidelvalle pushed a commit to danidelvalle/airbyte that referenced this pull request Feb 9, 2023
@teallarson @josephkmh @danidelvalle-frontiers
🪟 🧹 Credits page component cleanup (airbytehq#21870)
4dd8d30 
* update styling on connectionCell

* cleanup the component and get rid of custom styles where possible

* convert CreditsUsage page to scss module

* cleanup

* add connection status and connection name to endpoint data types

* use variable

* remove unused line

* align icon and name center

* adjust styling for arrow cell

---------

Co-authored-by: Joey Marshment-Howell <josephkmh@users.noreply.github.com>
This was referenced Feb 10, 2023
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@josephkmh josephkmh josephkmh approved these changes

@ambirdsall ambirdsall Awaiting requested review from ambirdsall

Assignees
No one assigned
Labels
area/frontend Related to the Airbyte webapp
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@teallarson @josephkmh @octavia-squidington-iv