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

New design for the widgets section in the docs #866

Merged
merged 14 commits into from
Jan 3, 2018
Merged

New design for the widgets section in the docs #866

merged 14 commits into from
Jan 3, 2018

Conversation

hdoro
Copy link
Contributor

@hdoro hdoro commented Dec 5, 2017

- Summary
A better way to display the widgets section that accommodates for further information and examples for each of the widgets.

- Description for the changelog
In order to allow the widgets' information to be edited via markdown, I had to create a new "widgets.html" partial that fits into the "widgets.html" default layout (I will, however, include a conditional in the current "single.html" template to include the partial there and make the code more maintainable). This partial pulls in all the markdown files with type: "widget" in the front matter (located in the folder site/content/docs/widgets) and renders them properly, but in the meantime I've only added the input and select fields as a proof of concept.

I've avoided conflicts with the current code by adding new files instead of changing existing ones, but when we're ready to merge I'll make sure the code isn't as bloated as it's now.

The CSS for the panels transition wasn't created, yet, but the JS groundwork is already layed out - and open for discussion, see site/static/widgets.js!

EDIT
You can check out how it's starting to look here, noting how the height jumping will be replaced by a smoother fading effect. I'll still work on accessibility and maybe change the visual aspect of it a bit - suggestions encouraged.

@verythorough
Copy link
Contributor

verythorough commented Dec 5, 2017

Deploy preview ready!

Built with commit b6ff20c

https://deploy-preview-866--netlify-cms-www.netlify.com

@erquhart
Copy link
Contributor

erquhart commented Dec 6, 2017

@hcavalieri just checked out the preview, this is really shaping up!!

@verythorough
Copy link
Contributor

verythorough commented Dec 6, 2017

@hcavalieri Very cool! As a stopgap for tomorrow, I'm putting all of the widget info into the existing doc. (You can see the preview, where I've added/updated everything but list and relation. (Ialso plan to remove the table at the top when I'm done.) Feel free to pull the info from that doc into your widget docs.

@hdoro
Copy link
Contributor Author

hdoro commented Dec 6, 2017

@verythorough I've used what you've done so far (including the WIP relation and list widgets) and improved the overall Hugo structure and the CSS feel of the section. Now there's a simple fade animation between each widget and a height variation for the grey BG of the section.
As far as I'm concerned, if you give an OK to the code and experience of this section, it's good to merge with the Widgets V2 and send it to production :) After finishing this PR I'll move to something else as soon as I have the time (I'm craving to create the radio and checkbox widgets, but I'm still a bit crude in React to do it quickly and efficiently)

PS: I had to make each widgets' title a h3 in order to avoid creating sub-navs on the sidebar, as I understood it was becoming too tall and clunky, and the widget cloud is already good enough for navigation. I could mess around with app.js and disable those specific links via JS, so if you want I could look into that.

@verythorough verythorough mentioned this pull request Dec 6, 2017
Copy link
Contributor

@verythorough verythorough left a comment

Choose a reason for hiding this comment

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

Very cool!

Using the h3 for widget names totally makes sense in the tag cloud layout. I used h2s in the full-text layout because the sidebar nav was a shortcut to navigating between widgets.

Also, I liked your original treatment of the term names ('Name:', 'UI:', etc. as bolded, and added it in a later commit to the PR. it's made it into some of your copies, but not all. Could you update so they all match?

Oh! And I realized this morning that I forgot the markdown widget! Would you like to write this one up?


The relation widget allows you to reference items from another collection. It provides a search input with a list of entries from the collection you're referencing, and the list automatically updates with matched entries based on what you've typed.

- **Name:** `realtion`
Copy link
Contributor

Choose a reason for hiding this comment

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

Oops! Looks like I made a typo that got copied into your version.

@@ -0,0 +1,31 @@
---
title: Widgets V2
position: 31
Copy link
Contributor

Choose a reason for hiding this comment

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

Of course, we'll want to updated the title and filename and remove the old widgets.md before merging.

pattern: ['.{10,}', "Must have at least 20 characters"]
```

## Check out all of the widgets
Copy link
Contributor

Choose a reason for hiding this comment

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

Especially because this is a nav item, I think we should choose a shorter, less conversational heading for this section, like "Built-in widgets" or "Widget options".

{{ end }}
</select>
</div>
</aside>
<article class="docs-content" id="docs-content">
{{- if eq .Title "Widgets V2" -}} <!-- Check if is widgets page, if so, add the partial "widgets" for the widget cloud functionality -->
Copy link
Contributor

Choose a reason for hiding this comment

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

Update this reference to match the title/filename change. :)

{{ end }}
</select>
</div>
</aside>
<article class="docs-content" id="docs-content">
{{- if eq .Title "Widgets V2" -}} <!-- Check if is widgets page, if so, add the partial "widgets" for the widget cloud functionality -->
{{ partial "edit-link" . }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can move the "edit-link" partial before the conditional, so you don't have to repeat it.

@@ -21,5 +21,8 @@
debug: false // Set debug to true if you want to inspect the dropdown
});
</script>
{{- if eq .Title "Widgets V2" -}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Update title ref here, too :)

{{ partial "edit-link" . }}
<h1 id="widgets">Widgets</h1>
{{ .Content }}
{{- partial "widgets" . -}}
Copy link
Contributor

Choose a reason for hiding this comment

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

We may need to change this later if we want to have content after the widgets partial. Down the line, we may want to abstract this whole tag-cloud pattern to a shortcode we can add anywhere, to any doc. Totally fine for now, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright! I'm a bit clueless with Hugo's shortcodes, but I'm up for the challenge! But I honestly don't know if there's any way to split a content's markdown... We could create two widgets.md files - one for the top content and one for the bottom. Idk, let me know what you think, @verythorough :D

- "Widgets define the data type and interface for entry fields. Netlify CMS comes with several built-in widgets. Click the widget names in the sidebar to jump to specific widget details. We’re always adding new widgets, and you can also [create your own](https://www.netlifycms.org/docs/custom-widgets)!"
- "Widgets are specified as collection fields in the `config.yml` file. Note that [YAML syntax](https://en.wikipedia.org/wiki/YAML#Basic_components) allows lists and objects to be written in block or inline style, and the code samples below include a mix of both."
- "To see working examples of all of the built-in widgets, try making a 'Kitchen Sink' collection item on the [CMS demo site](https://cms-demo.netlify.com). (No login required: click the login button and the CMS will open.) You can refer to the demo [configuration code](https://github.com/netlify/netlify-cms/blob/master/example/config.yml#L60) to see how each field was configured."
---
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like you don't need this paragraphs field anymore.

@verythorough
Copy link
Contributor

verythorough commented Dec 12, 2017

Deploy preview ready!

Built with commit b6ff20c

https://deploy-preview-866--cms-demo.netlify.com

Copy link
Contributor Author

@hdoro hdoro left a comment

Choose a reason for hiding this comment

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

Sorry for another commit, turns out I got really confused with all the nuances of git and spent a good half an hour smashing my head to the keyboard hahaha @verythorough it should be good to go now!

Copy link
Contributor

@verythorough verythorough left a comment

Choose a reason for hiding this comment

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

I'm so excited about this! I have just a few more nit-picks.

@media screen and (max-width: $tablet) {
#list, #relation, #object, #text, #string {
padding-bottom: 3rem;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious why you added this padding to some widgets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whenever there's a code block at the end of a widgets' card and either the screen is too narrow or the card is too long, the background is overflowed by the codeblock, so a quick and dirty hack I've found was to add extra padding to the bottom of it... Honestly idk how else to work around this, as I couldn't find a proper solution. Ideas?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, got it. Looking more closely at that aspect, I think you could fix it by removing the widgets__background span, and apply the background color and border radius styles to widgets__container instead. It seems to work fine for me when I test it in dev tools, and saves a bunch of css & js code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did the widgets__background span to grant a height transition effect to it as it wasn't happening without it, but I didn't try removing it after everything was done, turns out all is great now :) Super responsive as well, btw!

- **Options:**
- `default`: accepts `true` or `false`; defaults to `false`
- Example:

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it reads a little more clearly if Example is bold as well. It's bold in some widgets, but not in others.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

{{ partial "edit-link" . }}
{{ .Content }}
{{- end -}}
Copy link
Contributor

Choose a reason for hiding this comment

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

You can shorten lines 28-36 a bit by moving the conditional:

 	{{ partial "edit-link" . }}
	{{ .Content }}
	{{- if eq .Title "Widgets" -}} <!-- Check if is widgets page, if so, add the partial "widgets" for the widget cloud functionality -->
 		{{- partial "widgets" . -}} 
 	{{- end -}}

In conjunction with that, I'd add the h1 title to the beginning of the widgets.md file, so it matches the pattern of all of the other widgets (and then you don't need to include it here, either).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Awesome suggestion, done :D


// Widgets section
function widgetsCloud() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious why you moved this from a separate script called from the widgets page into the main js.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My point was to reduce the number of requests made, but I'll revert this back to widgets.js, no problem!

@talves
Copy link
Collaborator

talves commented Dec 14, 2017

This is a fantastic addition to the docs! Great job @hcavalieri

Copy link
Contributor Author

@hdoro hdoro left a comment

Choose a reason for hiding this comment

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

Alright, all polished up!

I've added the markdown widget and now we just have to solve the merge conflict and it should be good to go! Anything left, @verythorough ?

@hdoro hdoro changed the title WIP - New design for the widgets section in the docs New design for the widgets section in the docs Dec 14, 2017
@verythorough
Copy link
Contributor

I am really, really happy with how all of this came out (including the awesome new markdown widget section!) Thank you so much for this improvement.

I've set my approval. I think at this point we just need to check in with @erquhart about how best to handle the rebasing/conflicts and get merged! 🎉

@erquhart
Copy link
Contributor

erquhart commented Dec 15, 2017

Awesome work @hcavalieri!!! I think the only thing missing here is the ability to link to individual widget docs - it needs to be URL driven. If we use the url hash to store state, and convert the cloud buttons to links (and add an active class based on the url hash), should be good to go. Are you up for making that update?

Copy link
Contributor Author

@hdoro hdoro left a comment

Choose a reason for hiding this comment

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

There we go, @erquhart !

This was my first time playing around with URLs, so if you could review the changes to the widgets.js file and give me some feedback on it, it'd be amazing 😄

As always, thank you so much for your time and effort on this project, people!

@erquhart
Copy link
Contributor

Thanks @hcavalieri!

Going to open a separate issue to improve the url handling and get history working, but I'm good with merging this as is and improving from there.

Note that this still needs a rebase.

@tech4him1
Copy link
Contributor

@hcavalieri The contributor addition will probably need re-run after the rebase as well.

@hdoro
Copy link
Contributor Author

hdoro commented Dec 19, 2017

I'm sorry for the ignorance, but what should I do now for the merging? Just rebasing?

And @erquhart , the way I set the widgets up, you can check each one individually at their own page (see example), but I don't know how useful that'd be... Honestly this architecture is not optimal as it creates duplicate content, but I don't know how to make Hugo avoid rendering the widgets markdown files.

Overall, however, I think the benefits of having individual markdown files for each widget and its ease for maintainability is worth this drawback.

@tech4him1
Copy link
Contributor

As far as I know, it just needs a rebase. We'll work on the separate file issue afterwards.

@tech4him1
Copy link
Contributor

@hcavalieri Is this ready to merge?

@hdoro
Copy link
Contributor Author

hdoro commented Jan 3, 2018

@tech4him1 sorry for taking so long to answer! I've been without internet for the past couple of weeks...

It should be ready to merge, maybe the contributors file is kinda old as I rebased 14 days ago, let me know and I'll do it again :)

Copy link
Contributor

@tech4him1 tech4him1 left a comment

Choose a reason for hiding this comment

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

LGTM

```yaml
- {label: "Description", name: "description", widget: "text"}
```
## Default widgets
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder about using "Included widgets" or something like that instead of "Default".

@tech4him1
Copy link
Contributor

tech4him1 commented Jan 3, 2018

@hcavalieri No, the file doesn't need changed. It looks good to me!

@tech4him1 tech4him1 merged commit 1167f27 into decaporg:master Jan 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants