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

Improve story container #13

Closed
Mera-Gangapersaud opened this issue Nov 9, 2018 · 12 comments
Closed

Improve story container #13

Mera-Gangapersaud opened this issue Nov 9, 2018 · 12 comments
Labels
enhancement 🌈 New feature or request good first bug 🐛 Good for newcomers mentor:assigned 🦄 This issue has a dedicated mentor for guidance

Comments

@Mera-Gangapersaud
Copy link
Owner

The text editors sends its text to the story-container however when there is more text than space it overflows and does not scroll nicely.

Potential fix may be to change the formating of the div to make the story more readable

edit

@seanprashad
Copy link
Contributor

seanprashad commented Nov 9, 2018

The respective css for the container can be found here! 😼

I'd recommend to make it a textarea that has add a vertical scrollbar to the <div>.

I'm also available to mentor new comers for this issue - please give us a shout if you want to take it on! 😸

@Mera-Gangapersaud Mera-Gangapersaud added the enhancement 🌈 New feature or request label Nov 9, 2018
@seanprashad seanprashad added good first bug 🐛 Good for newcomers mentor:assigned 🦄 This issue has a dedicated mentor for guidance labels Nov 9, 2018
@deepanjali19
Copy link
Contributor

Hey @Mera-Gangapersaud and @seanprashad I would like to work on this issue. It great that this issue has a mentor assigned, as I am not too familiar with React.

@Mera-Gangapersaud
Copy link
Owner Author

Thanks for tackling this one @deepanjali19 Feel free to ask us questions here or reach out to us on Slack

@vldmrkl
Copy link
Collaborator

vldmrkl commented Nov 10, 2018

@deepanjali19 let me know if you run into issues with React, I can help as well.

@deepanjali19
Copy link
Contributor

Thanks @klymenkoo ! I would definitely ask :)

@seanprashad
Copy link
Contributor

seanprashad commented Nov 10, 2018

Love that you want to tackle this, despite not being familiar with React @deepanjali19 🙂We'll help you navigate a possible solution for this bug!

Figuring out a solution

To get things started, this is the <div> that we want to have a vertical scrollbar for:

<div className="story-container" dangerouslySetInnerHTML={{ __html: this.state.story }} />

You'll notice it has a className (equivalent to class from regular HTML) of story-container - lets check out the CSS applied to it (found in src/CreativeCollab.css):

.story-container {
position: absolute;
border: 1px solid black;
padding-left: 1%;
bottom: 5%;
left: 20%;
width: 800px;
height: 380px;
}

There's a few things that we can do here:

  1. We should add a css property for overflow. This will cause a scrollbar to appear when the contents exceed the height of the <div>. (Extra hint: Look at the documentation for overflow-y 😉)

  2. We can remove these properties from the story-container class:

-  position: absolute;
-  padding-left: 1%;
-  bottom: 5%;
-  left: 20%;

and update the height from 380px; to 340px;.

  1. We can also move it in between the two editors where users author their story. I'll leave this up to you to figure out, but here's what the final product it might look like:

story_container_final_image

@deepanjali19
Copy link
Contributor

Thank you so much everyone! :)

@tienpv222
Copy link

tienpv222 commented Nov 11, 2018

I think the CreativeCollab.jsx is just a test file. The real game interface shouldn't look like that because it has many probs, for ex: number of users allowed is up to 10, why 2 editors? Is a player supposed to see the other player's editor, or just the output is enough?

After digging into the project for a while, I personally think we are going too fast, making it's hard for further development. What we are lacking of:

  • The workflow: current description is too general.
    • When users first get into the app, do they see the game board? No, they need to join a room. So what do they see first? Room list? Button to join? Create room button?
    • If they create rooms, what's next? Game board now? No, it should be a form with some kind of info, player name? Max number of player? Room name?
    • Now in game board, at first the host has to wait for other players. How do they know when the game starts? How do they know if it's their turns? Should we have a kind of notification bar or pop up? Now that will affect the interface a lot
    • So on...
  • The design: this I believe the orders should be: wireframes -> implementations, not the other way around. For ex we need something like this first, before starting to code:
    untitled
  • Now with the two drafts above, we can have a frame for the app, which is very important for collaborating works to be consistent. Our master branch is not well organized. The entry CreativeCollab.jsx is a test file? Should we use App component like other projects? The current App component is TicTacToe game, which is just for research purpose, why it is in master branch? Should we put all components in a folder? For testing, should we include the test to repository or put it in .gitignore? We can do something similar to vuetify, don't include the playground file in the source code, but put them in description of every PR. And so on...

In short words, I believe the app need to be well organized first. Issues like #5 or #9 are good example of making the app bones. This is the very first thing we should do before going for further developments. I'm currently working on #10, and found it quite hard to code because it might conflict with the works of other contributors.

Just some personal opinions 😋

P/S: #20 is exactly what I wanna say

seanprashad pushed a commit that referenced this issue Nov 11, 2018
* improved story container

* fixing conflict

* final work

* updating the branch

* update overflow properties
@seanprashad
Copy link
Contributor

As #19 landed, we can iterate on this initial layout in future PR's

@seanprashad
Copy link
Contributor

@pynnl Love the level of detail that you've gone into!

If it's ok, I'd like to open a new issue with your post to get some visible discussion going there - stay tuned.

@dnguneratne
Copy link
Contributor

I agree with @pynnl. A few points:

@seanprashad
Copy link
Contributor

I hear ya @dnguneratne - I've made #21 fresh so it's easier for everyone to see and discuss. I'm going to put your comment over there!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🌈 New feature or request good first bug 🐛 Good for newcomers mentor:assigned 🦄 This issue has a dedicated mentor for guidance
Projects
None yet
Development

No branches or pull requests

6 participants