Skip to content

Fleet UI: Calendar settings iterations#17779

Merged
RachelElysia merged 3 commits into17230-calendar-featurefrom
calendar-settings-2
Mar 22, 2024
Merged

Fleet UI: Calendar settings iterations#17779
RachelElysia merged 3 commits into17230-calendar-featurefrom
calendar-settings-2

Conversation

@RachelElysia
Copy link
Member

@RachelElysia RachelElysia commented Mar 21, 2024

Issue

Iteration of #17444

Description

  • Add JSON error handling to UI
  • Add escape characters to format placeholder text correctly
  • Several styling nits

Screenrecording

https://www.loom.com/share/e22095de4a7a44e69a155f11f08c1878?sid=1d1e0285-d96c-4281-9586-2ae742b10c65

Checklist for submitter

If some of the following don't apply, delete the relevant line.

  • Manual QA for all new/changed functionality

@codecov
Copy link

codecov bot commented Mar 21, 2024

Codecov Report

Attention: Patch coverage is 10.00000% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 65.23%. Comparing base (268dd18) to head (53a7ab1).
Report is 3 commits behind head on 17230-calendar-feature.

Files Patch % Lines
...min/IntegrationsPage/cards/Calendars/Calendars.tsx 10.00% 9 Missing ⚠️
Additional details and impacted files
@@                    Coverage Diff                     @@
##           17230-calendar-feature   #17779      +/-   ##
==========================================================
- Coverage                   65.24%   65.23%   -0.01%     
==========================================================
  Files                        1202     1202              
  Lines                      109371   109380       +9     
  Branches                     2587     2590       +3     
==========================================================
+ Hits                        71358    71359       +1     
- Misses                      32598    32606       +8     
  Partials                     5415     5415              
Flag Coverage Δ
frontend 51.48% <10.00%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@RachelElysia RachelElysia marked this pull request as ready for review March 22, 2024 14:40
@RachelElysia RachelElysia requested a review from a team as a code owner March 22, 2024 14:40
Copy link
Contributor

@jacobshandling jacobshandling 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 it's worth it to work within the current error typing paradigm using type predicates instead of updating the global typescript rule.

Also this color shouldn't be greyed, since it's not placeholder text but the actual value to copy:
Screenshot 2024-03-22 at 10 43 04 AM

}

// Used to surface error.message in UI of unknown error type
type ErrorWithMessage = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice. This is probably something we can have as a utility type/typeguard for use across the UI.

@RachelElysia RachelElysia merged commit c558c14 into 17230-calendar-feature Mar 22, 2024
@RachelElysia RachelElysia deleted the calendar-settings-2 branch March 22, 2024 18:52
@RachelElysia RachelElysia restored the calendar-settings-2 branch March 26, 2024 20:18
@RachelElysia RachelElysia deleted the calendar-settings-2 branch March 26, 2024 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants