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

UX Squished TinyMCE Editor #586

Closed
wants to merge 2 commits into from

Conversation

muskie9
Copy link
Contributor

@muskie9 muskie9 commented May 17, 2019

resolves #485

This didn't turn out exactly as noted in the issue ticket from when it was last fixed. Below is the UX with adding the label. I played with ->addExtraClass('stacked') a bit but that didn't seem to change anything.

Screen+Shot+2019-05-16+at+9 25 54+PM

@muskie9
Copy link
Contributor Author

muskie9 commented May 17, 2019

I'm not a css guru, but I was playing around a bit and this may be where a bit of padding could happen:

.custom-summary {
  .ui-accordion-content,
  .ui-accordion-content .field {
    padding: 0;
  }

  // Change the caret to a plus icon
  .ui-icon-triangle-1-e {
    background-position: -16px -128px;
  }
}

I'm not sure what design patterns need to be followed and I don't have the FE build tools setup at this point.

Copy link
Member

@Cheddam Cheddam left a comment

Choose a reason for hiding this comment

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

@muskie9 Thanks for raising this, and sorry it hasn't received attention until now.

Had a play around with this PR, and it doesn't seem to render the way your screenshot suggests unless I add the $summary->addExtraClass('stacked'); you described. The added padding also seems to trigger horizontal scroll, and the layout actually looks fine without it.

If you've got some time to get this across the line, my recommendations are:

  • Target the PR at the 3 branch (git rebase --onto=3 HEAD~2)
  • Add the $summary->addExtraClass('stacked'); call
  • Remove the added padding

If not, I'll need to close this out.

@emteknetnz
Copy link
Member

It seems like there's going to be no further activity on this pull request, so we've closed it for now. Please open a new pull-request if you want to re-approach this work, or for anything else you think could be fixed or improved.

@emteknetnz emteknetnz closed this Sep 2, 2020
@lerni
Copy link
Contributor

lerni commented Dec 20, 2020

code bellow gets what the screenshot above shows with just css or also with stacked.

vendor/silverstripe/blog/client/src/styles/cms.scss:140

.custom-summary {
  .ui-accordion-content,
  .ui-accordion-content .field {
    padding: 0;

>     margin: 0;
>     &:last-of-type::after {
>       display: none;
>     }
>     .form__field-holder {
>       margin-left: 0;
>       max-width: calc(100% - 2px);
>       flex: initial;
>       padding-left: 0;
>       padding-right: 0;
>     }

    // WITH „stacked“
    // margin: 0;
    // &:last-of-type::after {
    //   display: none;
    // }
    // .form__field-holder {
    //   margin-left: 0;
    //   padding-left: 0;
    //   padding-right: 0;
    //   .mce-container {
    //     max-width: calc(100% - 2px);
    //   }
    // }
  }

  // Change the caret to a plus icon
  .ui-icon-triangle-1-e {
    background-position: -16px -128px;
  }
}

Haven’t got whats up with peerDependencies - on build yarn says bellow. To bring this further I would need know if you're OK with it - I would suggest withoutstacked and how to get jquery right in to the build. @NightJar helped me (also installed admin with yarn build env)

…/vendor/silverstripe/blog/client/src/bundles/cms.js
  2:20  error  Unable to resolve path to module ‚jquery‘  import/no-unresolved
  2:20  error  Missing file extension for „jquery“        import/extensions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Squished TinyMCE Editor
5 participants