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

refactor(sidebar state): moved onto TinaCMS #649

Merged
merged 1 commit into from Jan 22, 2020

Conversation

ncphillips
Copy link
Contributor

Looking for everyone's thoughts on this.

I started mulling over the Sidebar state after checking out @weibenfalk's thoughts on configuring the text on the Save/Reset buttons.

This change reifies the sidebar state and places it on the TinaCMS class as a property. I think this approach has a couple benefits:

  1. The API feels more righter
  2. It removes the need for the SidebarContext in the React app, instead relying on the pre-existinig CMSContext.

Here is how it would be used.

Checking sidebar state:

cms.sidebar.isOpen

Changing sidebar state:

cms.sidebar.isOpen = true

Subscribing Option 1

import { useCMS, useSubscribable } from "tinacms"
const cms = useCMS()

useSubscribable(cms.sidebar)

Subscribing Option 2:

const sidebar = useSidebar()

This change reifies the sidebar state and places
it on the TinaCMS class as a property. It also
removes the need for the `SidebarContext`
in the React app, instead relying on the
pre-existinig CMSContext.

Here is how it would be used.

Checking sidebar state:

    cms.sidebar.isOpen

Changing sidebar state:

    cms.sidebar.isOpen = true

Subscribing Option 1

    import { useCMS, useSubscribable } from "tinacms"
    const cms = useCMS()

    useSubscribable(cms.sidebar)

Subscribing Option 2:

    const sidebar = useSidebar()
@ncphillips ncphillips added the refactor Structural and semantic improvements label Jan 21, 2020
@github-actions
Copy link
Contributor

Warnings
⚠️

tinacms may need new tests.

⚠️ @tinacms/dev please add to a Milestone before merging
⚠️ Update Docs for #649

Create Issue

Modified Packages

The following packages were modified by this pull request:

  • tinacms

Generated by 🚫 dangerJS against cc8b9ad

@weibenfalk
Copy link
Contributor

I think it's great to remove the SidebarContext when it's not needed. One question .. how can you use "private" in the JS Class? Thought private wasn't supported. =) Isn't a private field still experimental and it uses # instead?

@ncphillips
Copy link
Contributor Author

It's a Typescript feature.

The ECMAscript private fields syntax is coming in 3.8, but we're not on that yet.

@weibenfalk
Copy link
Contributor

ahhh ok ... that explains things. Great, thanks for the info.

@ncphillips ncphillips added this to the 2020-01-27 Release milestone Jan 22, 2020
@ncphillips ncphillips merged commit a0b542e into master Jan 22, 2020
@ncphillips ncphillips deleted the refactor-cms-sidebar-state branch January 22, 2020 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Structural and semantic improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants