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 UI #2920

Merged
merged 10 commits into from
Jul 7, 2020
Merged

Dashboard: Settings UI #2920

merged 10 commits into from
Jul 7, 2020

Conversation

BrittanyIRL
Copy link
Contributor

Summary

Lays groundwork for rest of settings page within the dashboard - file structure and the base UI brought in from Figma.

Screen Shot 2020-07-01 at 2 48 04 PM

Relevant Technical Choices

  • Adds views/editorSettings to dashboard/app. All work is visible in storybook alone right now to avoid messing with redundant feature flags and keep PRs smaller.
  • Separates GA and publisher logo into their own directories w/ components for each.
  • Sets up text
  • Sets up styled components
  • Used theme from dashboard as much as possible, everywhere I didn't it's because these are different values from what the rest of the dashboard is using - considering the UI that Sam's working on I'm not worried about this as I know changes will be fluid.
  • Took small liberties around how wide the main (form) sections of this view will be so that resizing is graceful - figma shows the containers at 600px but the full size of the window is 1884px wide which is massive.

To-do

(These will all be separate)

  • Upload component
  • Adding link to text in publisherLogo instructions (need to find out what the link is, have a todo note for later)
  • All functionality 🙃
  • Tests
  • Adding view inside of dashboard

User-facing changes

  • None

Testing Instructions


Addresses #2747

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

codecov bot commented Jul 1, 2020

Codecov Report

Merging #2920 into master will decrease coverage by 0.33%.
The diff coverage is 58.62%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2920      +/-   ##
==========================================
- Coverage   81.49%   81.15%   -0.34%     
==========================================
  Files         704      714      +10     
  Lines       11953    12016      +63     
==========================================
+ Hits         9741     9752      +11     
- Misses       2212     2264      +52     
Flag Coverage Δ
#karmatests 59.55% <51.72%> (-0.02%) ⬇️
#unittests 66.54% <10.34%> (+0.03%) ⬆️
Impacted Files Coverage Δ
assets/src/dashboard/app/index.js 100.00% <ø> (ø)
...ts/src/dashboard/app/views/editorSettings/index.js 0.00% <0.00%> (ø)
.../src/dashboard/components/inlineInputForm/index.js 88.88% <ø> (ø)
.../app/views/editorSettings/googleAnalytics/index.js 50.00% <50.00%> (ø)
...rd/app/views/editorSettings/publisherLogo/index.js 50.00% <50.00%> (ø)
...c/dashboard/app/views/editorSettings/components.js 57.14% <57.14%> (ø)
.../src/dashboard/app/views/myStories/header/index.js 85.71% <100.00%> (+1.09%) ⬆️
includes/REST_API/Stories_Base_Controller.php 61.81% <0.00%> (-34.55%) ⬇️
...s/src/edit-story/elements/media/useAverageColor.js 78.57% <0.00%> (-14.29%) ⬇️
includes/REST_API/Stories_Controller.php 66.94% <0.00%> (-13.96%) ⬇️
... and 26 more

@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2020

Size Change: +1 kB (0%)

Total Size: 1 MB

Filename Size Change
assets/js/edit-story.js 458 kB +228 B (0%)
assets/js/stories-dashboard.js 527 kB +776 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/web-stories-embed-block.js 15.7 kB 0 B

compressed-size-action

Comment on lines -128 to +136
handleSortChange={useCallback(
(newSort) => {
sort.set(newSort);
scrollToTop();
},
[scrollToTop, sort]
)}
handleSortChange={onSortChange}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is just a nit that pascal asked for yesterday in the rush for beta, making good on the update :D

@BrittanyIRL BrittanyIRL force-pushed the feature/2747-initial-ui branch from 7907198 to 7e9c097 Compare July 1, 2020 22:18
@BrittanyIRL BrittanyIRL force-pushed the feature/2747-initial-ui branch from b433126 to e239f98 Compare July 3, 2020 17:05
Copy link
Contributor

@maxyinger maxyinger left a comment

Choose a reason for hiding this comment

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

noice!

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 small nits for your to consider, but it looks good!

<div>
<FileUploadHelperText>{TEXT.context}</FileUploadHelperText>
<UploadContainer>
<p>{'Upload Placeholder'}</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

How come "Upload Placeholder" isn't part of the TEXT const?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because it will go away when this section gets fleshed out to be a file uploader. It's literally just a placeholder visually right now.

@BrittanyIRL BrittanyIRL force-pushed the feature/2747-initial-ui branch from 7a12e6a to fae8cc6 Compare July 6, 2020 17:15
@BrittanyIRL BrittanyIRL requested a review from carloskelly13 July 7, 2020 18:00
Copy link
Contributor

@carloskelly13 carloskelly13 left a comment

Choose a reason for hiding this comment

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

This looks good with one minor caution about spreading props on an input.

@BrittanyIRL BrittanyIRL merged commit 5af83e5 into master Jul 7, 2020
@BrittanyIRL BrittanyIRL deleted the feature/2747-initial-ui branch July 7, 2020 19:48
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.

6 participants