Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Informations #249

Open
chack1172 opened this issue Aug 28, 2016 · 50 comments
Open

Informations #249

chack1172 opened this issue Aug 28, 2016 · 50 comments

Comments

@chack1172
Copy link
Contributor

Sorry guys this is not a issue but a question.
I'm changing admin.css file and then I have to do a pull request.
Do I have to change admin.css.map too? Do I have to change it manually?

@euantorano
Copy link
Member

Hi,

The CSS is generated from the SASS files in resources/sass. Please edit these files rather than the CSS, then run the Gulp tool. For more information, please see the wiki associated with this repository.

On 28 Aug 2016, at 20:01, Nereo Berardozzi [email protected] wrote:

Sorry guys this is not a issue but a question.
I'm changing admin.css file and then I have to do a pull request.
Do I have to change admin.css.map too? Do I have to change it manually?


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@chack1172
Copy link
Contributor Author

chack1172 commented Aug 28, 2016

Thank you for the response euantorano :)

The currect path is resources/assets/sass

@chack1172
Copy link
Contributor Author

chack1172 commented Aug 28, 2016

@euantorano I never used sass, is it correct?
Template:
<table class="admin-table admin-table--bordered users"> <thead> <tr> <th colspan="2">{{ trans('admin::users.avatar_field') }}</th> </tr> </thead> <tbody> {% for user in users %} <tr> <td class="avatar-cell"> <img src="{{ user.avatar }}"> </td> <td>{{ user.name }}</td> </tr> {% endfor %} </tbody> </table>

Style
`.admin-table {
width: 100%;

&.admin-table--bordered {
    border-radius: 5px;
    border-collapse: separate;

    td {
        border-bottom: 1px solid $border-1;
    }
}

td, th {
    padding: 10px;

    &.avatar-cell {
        width: 44px;

        img {
            width: 44px;
            height: 44px;
        }
    }
}

thead {
    background: $primary-color;
    color: $invert-font-color;
}

}`

@euantorano
Copy link
Member

Looks right to me, but I'll need to test it once you create a pull request 😄

On 28 Aug 2016, at 20:48, Nereo Berardozzi [email protected] wrote:

@euantorano I nevere used sass, is it correct?
Template:

{% for user in users %} {% endfor %}
{{ trans('admin::users.avatar_field') }}
{{ user.name }}

Style
`.admin-table {
width: 100%;

&.admin-table--bordered {
border-radius: 5px;
border-collapse: separate;

td {
    border-bottom: 1px solid $border-1;
}

}

td, th {
padding: 10px;

&.avatar-cell {
    width: 44px;

    img {
        width: 44px;
        height: 44px;
    }
}

}

thead {
background: $primary-color;
color: $invert-font-color;
}
}`


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@chack1172
Copy link
Contributor Author

You have to wait for the pull request, it is an hard edit for me :o never used laravel too

@euantorano
Copy link
Member

Not a problem, any help is appreciated!

On 28 Aug 2016, at 20:52, Nereo Berardozzi [email protected] wrote:

You have to wait for the pull request, it is an hard edit for me :o never used laravel too


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@chack1172
Copy link
Contributor Author

Ok I used gulp watch to change files.
There's a litte problem:
- resources\assets\sass\rtl.scss <-- Not Found

@euantorano
Copy link
Member

Interesting. I've maybe broken that one at some point. If you just commit your changed files and make a Pull Request I can fix it.

On 28 Aug 2016, at 21:39, Nereo Berardozzi [email protected] wrote:

Ok I used gulp watch to change files.
There's a litte problem:

  • resources\assets\sass\rtl.scss <-- Not Found


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@chack1172
Copy link
Contributor Author

chack1172 commented Aug 28, 2016

The problem is in the file gulfile.js at line 51
https://github.com/mybb/mybb2/blob/master/gulpfile.js

The file rtl.scss doen't exists, it was changed to main.rtl.css
https://github.com/mybb/mybb2/tree/master/resources/assets/sass

@euantorano
Copy link
Member

Yeah, the file should exist, unless it's been removed recently. It provides right to left support for text in the ACP. I'll look into it, but feel free to just submit a PR with the sass.

On 28 Aug 2016, at 22:01, Nereo Berardozzi [email protected] wrote:

The problem is in the file gulfile.js at line 51
https://github.com/mybb/mybb2/blob/master/gulpfile.js

The file rtl.scss doen't exists
https://github.com/mybb/mybb2/tree/master/resources/assets/sass


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@chack1172 chack1172 changed the title map file Informations Aug 29, 2016
@chack1172
Copy link
Contributor Author

chack1172 commented Aug 29, 2016

How ho I set a default value to form_select if a variable doesn't exists?

{{ form_select('role', roles, role ? role.id) }}

Solved

@euantorano
Copy link
Member

Hi, the third parameter is the default value (as seen here). I'm not sure what it is that you're trying to do, but the role ? role.id will set the default to the role's ID if the role isn't null.

@chack1172
Copy link
Contributor Author

I meant if role was null, but now I know how to do that:

{{ form_select('role', roles, role ? role.role_slug : 'user') }}

@chack1172
Copy link
Contributor Author

When I commit the changes, do I have to update admin.css, admin.css.map, admin_rtl.css and admin_rtl.css.map?

@euantorano
Copy link
Member

euantorano commented Aug 29, 2016

Hi,

Gulp will do that, but it's broken at the minute. If you just commit the SASS, I can generate the CSS and map files.

@chack1172
Copy link
Contributor Author

I'm trying to send a pull request but it select my past commit and devilshakerz commit too. How could I send only my last commit of my fork?

@euantorano
Copy link
Member

Hi,

Did you create a new branch before starting doing your work? That's the way that we recommend doing things.

@chack1172
Copy link
Contributor Author

No, I didn't. I didn't know that

@chack1172
Copy link
Contributor Author

I revert changes and create a new brach

@euantorano
Copy link
Member

Ah, we should probably make our recommendations a bit more obvious, perhaps in the README file.

On 29 Aug 2016, at 17:25, Nereo Berardozzi [email protected] wrote:

No, I didn't. I didn't know that


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@chack1172
Copy link
Contributor Author

Finally I've done it :) Now how do I update my master with edits?

@euantorano
Copy link
Member

euantorano commented Aug 29, 2016

Hi,

If you push your branch, then you should be able to create a pull request from that branch to mybb2/master 😄

@chack1172
Copy link
Contributor Author

Ok. I've done the pull request

@chack1172
Copy link
Contributor Author

For validation do I have to do something like the page below?
https://github.com/mybb/mybb2/blob/master/app/Http/Requests/Account/UpdateProfileRequest.php

@chack1172
Copy link
Contributor Author

@euantorano can you help me updating my fork with latest edits? I tried many times but it adds a new commit:
https://github.com/chack1172/mybb2/commits/master

@euantorano
Copy link
Member

Hi,

You need to do a rebase rather than a pull. For more information, please see here: https://robots.thoughtbot.com/keeping-a-github-fork-updated

We need to really document this stuff properly. I'll try and put out a dev post on our blog about our recommended workflow this week.

@chack1172
Copy link
Contributor Author

@eyabtirabi how do I use Request validation for this? I need the id
`$user = $this->userRepository->find($id);

    $validator = $this->validator->make($request->all(), [
        'name'      => 'required|max:255|unique:users,name,'.$user->id,
        'email'     => 'required|email|max:255|unique:users,email,'.$user->id,
        'password'  => 'confirmed|min:6',
        'usertitle' => 'string',
    ]);`

@euantorano
Copy link
Member

Hi,

Is the user the current logged in user, or the user being edited? If it's the latter, you can use to IoC container to inject the user repository or model and look the user up. If you want, I can do it for you tomorrow.

@chack1172
Copy link
Contributor Author

user being edited.
Could I have an example?

@euantorano
Copy link
Member

Hi,

There are none in the core, I would just leave it as it is for now and I'll fix it later and show you what I mean 😄

@chack1172
Copy link
Contributor Author

Maybe I know how to do that or maybe not xD I'll let you know

@chack1172
Copy link
Contributor Author

chack1172 commented Aug 31, 2016

It show me this error when i post edit user form :(
Class MyBB\Core\Http\Requests\User\SaveUserRequest does not exist

In UserController I inserted this:
use MyBB\Core\Http\Requests\User\SaveUserRequest;

SaveUserRequest.php:
<?php
/**
* User edit request.
*
* @author MyBB Group
* @version 2.0.0
* @package mybb/core
* @license http://www.mybb.com/licenses/bsd3 BSD-3
*/

namespace MyBB\Core\Http\Requests\User;

use MyBB\Core\Http\Requests\AbstractRequest;

class SaveUserRequest extends AbstractRequest
{
/**
* @return array
*/
public function rules()
{
return [
'name' => 'required|max:255|unique:users',
'email' => 'required|email|max:255|unique:users',
'password' => 'confirmed|min:6',
'usertitle' => 'string',
]:
}

/**
* @return bool
*/
public function authorize()
{
return true;
}
}

@euantorano
Copy link
Member

euantorano commented Sep 1, 2016

Hi,

Did you run composer dump-autoload?

@chack1172
Copy link
Contributor Author

There was a problem in rules function, I fixed it before send the pull request.
What do you think about the new create user request?

@euantorano
Copy link
Member

Looks good to me. We did notice that editing a user will set their password to an empty string though if you don't include one. This line: https://github.com/mybb/mybb2/blob/master/app/Http/Controllers/Admin/Users/UserController.php#L94

Should be:

$password = '';
if ($request->has('password')) {
    $password = $request->input('password');
}

$values = $request->only('name', 'email', 'usertitle');

if (!empty($password)) {
    $values['password'] = bcrypt($password);
}

$values['updated_at'] = $user->freshTimestamp();

$user->update($values);

Or something like that, anyway. I haven't tested it yet 😄

@chack1172
Copy link
Contributor Author

I forgot to test it.
Today I'll test and modify it

@euantorano
Copy link
Member

Nice one, thanks!

@chack1172
Copy link
Contributor Author

Hei @euantorano $user->freshTimestamp(); doesn't work. updated_at isn't updated

@Matslom
Copy link
Contributor

Matslom commented Sep 1, 2016

Hi @chack1172 use $user->setUpdatedAt($user->freshTimestamp())->save(); it should work.

@chack1172
Copy link
Contributor Author

Hi @Matslom thank you, I saw now that updated_at isn't in fillable array

@chack1172
Copy link
Contributor Author

I updated mybb2 in my local website and now in console there's this error:
ReferenceError: Modernizr is not defined

I tried to use the command composer require modernizr with cmd and it show me this error:
[InvalidArgumentException]
Could not find package modernizr at any version for your minimum-stability (dev). Check the package spelling or your minimum-stability

@euantorano
Copy link
Member

Hi, yeah that's a known issue (see the TODO: modernizr... in gulpfile.js).

I'm looking at the best way to use modernizr. We don't need the whole modernizr library (if at all? Paging @justinsoltesz and @Eric-Jackson...), only some of the components.

@chack1172
Copy link
Contributor Author

There's something that I can do to help you? I don't have ideas...

@euantorano
Copy link
Member

There's not much you can do to help with the modernizr issue, I'm not sure what your strengths are? Obviously you know at least a bit of PHP, are you any good with JS?

There's always work to do, if I know what you're best at, we can guide you to the right place 😄

@chack1172
Copy link
Contributor Author

Maybe it's better that I focus on php xD

@euantorano
Copy link
Member

Fair enough, there's plenty to do there too! One of the big jobs I will be doing soon is the move to Laravel 5.3, which I need to do. Other than that, there are a couple of big things that could be worked on:

  • Show the number of likes a user has made/received on their profile (this will need to look at forum permissions, once we've made them - for now it can be a simple count).
  • Reporting posts/threads/users/conversations (so similar to the warning system that @Matslom is working on, but slightly different).
  • Spam detection - integration with StopForumSpam and other projects. This is maybe best left until later?
  • Forum management in the ACP (the ACP is also going to get a proper design soon).
  • Attachments - I started work on this, but it's nowhere near done. Might be worth starting from scratch?

There's a lot more, but those pop into my head. You can also look at the suggestions forum on the forums too to get an idea of what's planned (I need to make this clearer at some point...).

If you choose something to work on, we can discuss it together and get a plan set so that you know what you're doing. I'm also usually available if you have any questions - either PM me on the forums (Euan T) or contact me on Discord (details on the forums, here).

@chack1172
Copy link
Contributor Author

Ok, thank you.

@euantorano
Copy link
Member

I believe you also have my email if you prefer that to talk and get things planned - GitHub notifications aren't very good for me (they don't always show up until a long time after something is commented).

@chack1172
Copy link
Contributor Author

All my staff use discord, it's ok :)

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

No branches or pull requests

3 participants