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.

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

Dashboard: Settings view context foundation (Google Analytics) #2976

Merged
merged 12 commits into from
Jul 8, 2020

Conversation

BrittanyIRL
Copy link
Contributor

@BrittanyIRL BrittanyIRL commented Jul 3, 2020

Summary

Setting up hooks for settings data

Relevant Technical Choices

  • Create new hook for useSettingsApi that will eventually give us the data we need for accessing google analytics ids (not part of default wordpress settings api)
  • Wire up event handlers using inline input component

To-do

  • Figure out a plan for getting the API endpoint necessary wired up (then wire it up and write tests for successful responses) (see here: Settings Data: Google Analytics  #3031)
  • Confirm how we want to handle "submit" actions in this feature. Right now it responds to 'enter' just like renaming a story.

User-facing changes

Invisible change at the moment

Testing Instructions

  • See unit tests are working properly

addresses #2747

@codecov
Copy link

codecov bot commented Jul 3, 2020

Codecov Report

Merging #2976 into master will decrease coverage by 0.70%.
The diff coverage is 63.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2976      +/-   ##
==========================================
- Coverage   81.87%   81.16%   -0.71%     
==========================================
  Files         690      716      +26     
  Lines       10464    12048    +1584     
==========================================
+ Hits         8567     9779    +1212     
- Misses       1897     2269     +372     
Flag Coverage Δ
#karmatests 59.46% <12.12%> (-0.16%) ⬇️
#unittests 66.52% <57.57%> (-0.03%) ⬇️
Impacted Files Coverage Δ
...ts/src/dashboard/app/views/editorSettings/index.js 0.00% <0.00%> (ø)
.../app/views/editorSettings/googleAnalytics/index.js 25.00% <16.66%> (-25.00%) ⬇️
assets/src/dashboard/app/api/useSettingsApi.js 81.25% <81.25%> (ø)
assets/src/dashboard/app/reducer/settings.js 83.33% <83.33%> (ø)
assets/src/dashboard/app/api/apiProvider.js 100.00% <100.00%> (ø)
...c/dashboard/app/views/editorSettings/components.js 57.14% <100.00%> (ø)
includes/REST_API/Link_Controller.php 93.24% <0.00%> (ø)
includes/REST_API/Stories_Autosaves_Controller.php 94.91% <0.00%> (ø)
includes/Traits/Types.php 0.00% <0.00%> (ø)
includes/Fonts.php 76.47% <0.00%> (ø)
... and 22 more

@BrittanyIRL BrittanyIRL added Group: Dashboard Pod: Dashboard & Templates Type: Enhancement New feature or improvement of an existing feature labels Jul 3, 2020
@BrittanyIRL BrittanyIRL self-assigned this Jul 3, 2020
@BrittanyIRL BrittanyIRL added this to the Sprint 32 milestone Jul 3, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2020

Size Change: +283 B (0%)

Total Size: 1 MB

Filename Size Change
assets/js/stories-dashboard.js 528 kB +283 B (0%)
ℹ️ View Unchanged
Filename Size Change
assets/css/edit-story.css 269 B 0 B
assets/css/stories-dashboard.css 305 B 0 B
assets/css/web-stories-embed-block.css 515 B 0 B
assets/js/edit-story.js 458 kB 0 B
assets/js/web-stories-embed-block.js 15.7 kB 0 B

compressed-size-action

@BrittanyIRL BrittanyIRL force-pushed the feature/2747-initial-ui branch from 7a12e6a to fae8cc6 Compare July 6, 2020 17:15
@BrittanyIRL BrittanyIRL force-pushed the feature/2747-wiring-ga-functionality branch from 5f7b01e to c780811 Compare July 6, 2020 17:22
Base automatically changed from feature/2747-initial-ui to master July 7, 2020 19:48
@BrittanyIRL BrittanyIRL force-pushed the feature/2747-wiring-ga-functionality branch from adcbe52 to eaf5d1f Compare July 7, 2020 20:18
@BrittanyIRL BrittanyIRL marked this pull request as ready for review July 7, 2020 20:24
Copy link
Contributor

@mariano-formidable mariano-formidable left a comment

Choose a reason for hiding this comment

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

I left a comment for you to consider 🤔

assets/src/dashboard/app/api/useSettingsApi.js Outdated Show resolved Hide resolved
@BrittanyIRL BrittanyIRL force-pushed the feature/2747-wiring-ga-functionality branch from 7d74ddb to b295ba3 Compare July 8, 2020 00:35
Copy link
Contributor

@mariano-formidable mariano-formidable left a comment

Choose a reason for hiding this comment

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

Only 1 tiny itty bitty nit that you can freely ignore. Excellent work!

assets/src/dashboard/app/api/useSettingsApi.js Outdated Show resolved Hide resolved
@BrittanyIRL BrittanyIRL force-pushed the feature/2747-wiring-ga-functionality branch from b295ba3 to e2c4369 Compare July 8, 2020 15:54
@BrittanyIRL BrittanyIRL merged commit aa5fff8 into master Jul 8, 2020
@BrittanyIRL BrittanyIRL deleted the feature/2747-wiring-ga-functionality branch July 8, 2020 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Group: Dashboard Type: Enhancement New feature or improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants