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

Implement form elements for the design panel #271

Merged
merged 34 commits into from
Mar 3, 2020
Merged

Conversation

wassgha
Copy link
Contributor

@wassgha wassgha commented Feb 10, 2020

Addresses #255

Screen Shot 2020-02-09 at 10 48 11 PM

Changes

  • Implements the following form elements for use in the design panel
    • Numeric input
    • Input rows
    • One-state and two-state toggle buttons
    • Spacer
    • Color input (to be used with Custom color picker #180 )
  • Simple UI for color presets (non-functional)
  • Correctly styles the design panel's headers
  • Merges the position, size and rotation panels into one panel
  • Uses icons for some actions instead of buttons
  • Design update based on new design
  • TextStyle panel update
  • TextInput
  • Unit tests added

To-do

/cc @dvoytenko

See #255

webpack.config.js Outdated Show resolved Hide resolved
@swissspidy swissspidy mentioned this pull request Feb 10, 2020
5 tasks
swissspidy added a commit that referenced this pull request Feb 10, 2020
@spacedmonkey
Copy link
Contributor

I think we should discuss the use of material-ui instead of implement our own here.

@dvoytenko
Copy link
Contributor

I think we should discuss the use of material-ui instead of implement our own here.

I agree. Let's give it a little time for us to review material libraries and material requirements.

assets/src/edit-story/components/canvas/pagemenu/index.js Outdated Show resolved Hide resolved
assets/src/edit-story/components/form/button.js Outdated Show resolved Hide resolved
assets/src/edit-story/components/form/button.js Outdated Show resolved Hide resolved
assets/src/edit-story/components/form/color.js Outdated Show resolved Hide resolved
assets/src/edit-story/components/form/color.js Outdated Show resolved Hide resolved
assets/src/edit-story/components/form/color.js Outdated Show resolved Hide resolved
@obetomuniz
Copy link
Contributor

BTW, @wassgha, just in order to complete the PR, can you please confirm to me and @ndev1991 the remaining issues for this ticket if there is something to complete? Mainly considering the To-do List: Is that accurate? Thx 👍.

@wassgha
Copy link
Contributor Author

wassgha commented Feb 20, 2020

Hey @obetomuniz , to close this let's just address the review comments and to recap:

  • Discuss whether attributes like expand and boxed are needed or should the user style the component externally (wrap it and add the flex: 1 style to it for example)
  • Discuss and either revert or keep the reorganization of icons (should icons be close to their components or should they be in a separate folder)
  • Refactor the Input styled component that's inside color.js, input.js, numeric.js (could probably re-use input.js and add some styles to it
  • Make the color input functional (alpha channel should somehow be inferred from the color, sync with @swissspidy on the color picker)

and to fully close #255 (probably using a separate PR since this one is getting big)

  • Style the text input correctly as in the Figma
  • Create a toggle switch component like the one used to toggle between Background Display Options (sync with Sam)
    Screen Shot 2020-02-20 at 1 42 27 PM
  • Add tooltips to the components
  • Re-organize the panels to look like the Figma
  • Create the combo box component
  • Style the image upload component correctly (for example for the video poster image)

@barklund
Copy link
Contributor

@obetomuniz @wassgha @ndev1991, some comments on the todo:

  • Add tooltips to the components

Not relevant for #255. #66 is separate issue all together.

  • Re-organize the panels to look like the Figma

Not relevant for #255. Just create the components themselves. Organizing the panels will come in several follow-up PR's.

@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

- Remove expand and boxed properties to promote wrap styling of core components
- Refactor input.js and re-use it on color.js and numeric.js
- Remove specific styles to promote wrap styling of core components
@obetomuniz
Copy link
Contributor

obetomuniz commented Feb 21, 2020

Hello, from what we brief in order to close this PR#271, I accomplished the following:

  • Removed expand and boxed properties to promote wrap styling of core components
  • Refactored input.js and re-use it on color.js and numeric.js(actually input.js was not being used)
  • Remove specific styles to promote wrap styling of core components

I'm now involved with the last one Make the color input functional (which is about to add alpha channel support on color picker). @swissspidy, as @wassgha mentioned you in this specific one, if you have inputs about that, I would love to know. In the meaning time, I am doing investigation from myself 👍

Copy link
Contributor

@barklund barklund left a comment

Choose a reason for hiding this comment

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

Very clean and nice component library. Great stuff!

assets/src/edit-story/components/form/switch.js Outdated Show resolved Hide resolved
assets/src/edit-story/components/form/switch.js Outdated Show resolved Hide resolved
assets/src/edit-story/components/form/switch.js Outdated Show resolved Hide resolved
assets/src/edit-story/components/form/switch.js Outdated Show resolved Hide resolved
assets/src/edit-story/components/panels/size.js Outdated Show resolved Hide resolved
assets/src/edit-story/components/panels/size.js Outdated Show resolved Hide resolved
assets/src/edit-story/components/panels/size.js Outdated Show resolved Hide resolved
assets/src/edit-story/components/panels/textStyle.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@obetomuniz
Copy link
Contributor

obetomuniz commented Feb 21, 2020

Hey @wassgha @swissspidy @barklund @ndev1991, given my research on the Make the color input functional supporting alpha channel issue, input's of type color only allow simple colors as described here, which means no support for alpha channel.

My initial suggestion is to use a Numeric Input side by side the Color Input instead of a plain percentage text label and allow the user to change that value in order to have the opacity representation of a given color. BTW, we can even retrieve #RRGGBBAA format, or as usual, RGBA. Let me know your thoughts and if there is any other workaround to support the alpha channel on it.

function hexToRGBA(color, opacity) {
  const R = parseInt(color.slice(-6, -4), 16);
  const G = parseInt(color.slice(-4, -2), 16);
  const B = parseInt(color.slice(-2), 16);

  return `rgba(${R}, ${G}, ${B}, ${opacity / 100})`;
}

function hexToRRGGBBAA(color, opacity) {
  const opacityRGBRef = Math.round((opacity / 100) * 255);
  const hex = opacityRGBRef.toString(16);

  return `#${color}${hex.padStart(2, "0").toUpperCase()}`;
}

hexToRGBA('000000', 80)          //   rgba(0, 0, 0, 0.8)
hexToRRGGBBAA('000000', 80)      //   #000000CC

@barklund
Copy link
Contributor

@googlebot I consent.

@barklund
Copy link
Contributor

@obetomuniz @ndev1991 remember to consent to Google CLA Bot (as you have contributed to a PR, you didn't originate) by simply typing @googlebot I consent. as a comment here.

@obetomuniz
Copy link
Contributor

@googlebot I consent.

@swissspidy swissspidy requested a review from miina March 2, 2020 17:53
Copy link
Contributor

@miina miina left a comment

Choose a reason for hiding this comment

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

Note that there are parallel PR-s for the functionality of the Design Panel parts which also do shuffling around the panels and adding missing functionality.

As long as the form elements all exist and are designed I'm happy to do all the refactoring and functionality fixes for the things that broke, in those PRs to get this merged faster and for having more smaller follow-up PRs instead. /cc @swissspidy

assets/src/edit-story/components/panels/colorPreset.js Outdated Show resolved Hide resolved
assets/src/edit-story/components/form/button.js Outdated Show resolved Hide resolved
assets/src/edit-story/components/form/numeric.js Outdated Show resolved Hide resolved
assets/src/edit-story/components/panels/test/actions.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
ndev1991 and others added 3 commits March 3, 2020 10:42
…design-panel

* 'design-panel' of github.com:google/web-stories-wp:
  remove duplicated external dependencies section
.eslintrc Outdated Show resolved Hide resolved
@obetomuniz
Copy link
Contributor

obetomuniz commented Mar 3, 2020

In order to accomplish potential improvements in this PR, since the current state is stable and pretty advanced, I would recommend approve/merge it ASAP. It's blocking other PRs, and like other parts of the current version of the plugin, it will require on-demand improvements that we can deal with separately in additional PRs.

@barklund barklund dismissed dvoytenko’s stale review March 3, 2020 21:20

We're merging as-is and following up in new PR's

@swissspidy swissspidy changed the title Implements form elements for the design panel Implement form elements for the design panel Mar 3, 2020
@swissspidy swissspidy merged commit bfa3aa2 into master Mar 3, 2020
@swissspidy swissspidy deleted the design-panel branch March 3, 2020 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement New feature or improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants