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

User menu #50

Merged
merged 12 commits into from
Sep 15, 2015
Merged

User menu #50

merged 12 commits into from
Sep 15, 2015

Conversation

claudiodekker
Copy link
Contributor

This should (hopefully) mark #45 as completed.

	- Moved away forgot password and register links, which were both obtained via config() method, to a more approriate place for easier future management.

Login Modal:
	- Completely removed and merged into the new user-dropdown object. (It was in a strange location to begin with)

Master View/Layout:
	- Fixed a bug where the login modal was included even when you were logged in.
	- Instead of including the login model, we're now 'yielding' it instead. (It's now located in the new user-dropdown object)

User-Dropdown Object:
	- New!
	- Contains logic for both logging in, and when we are actually logged in.
@nekodex
Copy link
Collaborator

nekodex commented Sep 5, 2015

Found an issue - if you repeatedly click on your avatar to show/hide the dialog too fast, the page progressively gets dimmer and dimmer.

@claudiodekker
Copy link
Contributor Author

@nekodex Well spotted. Turns out that's an issue already in existence long before this feature was added, namely with the login modal. (Look at the currently live site, which doesn't include this commit)

To be fair, I don't think this issue is that important, as it doesn't block user-experience, but I'll still take a look at it, as it should also resolve the issue on the guest-side of things.

If possible, I'll see if I can create a separate bugtracker-issue for this & claim it, although I'm not sure if I've got the rights to do so.

@nanaya
Copy link
Collaborator

nanaya commented Sep 5, 2015

  • ctb (and other play mode) logo is available as font <i class="fa osu fa-ctb-o"></i>
  • country flags are in images/flags/
  • check this regarding css: CSS Major Overhaul #53
  • apparently having the modal button in front of overlay breaks the overlay ;_;
  • try using localization when possible
  • don't worry about prefixing, we have autoprefixer for that (there's also usage of flexbox which means you don't need to worry about ie < 10). And presto is dead

@claudiodekker
Copy link
Contributor Author

@nanaya Alright, thanks. I'll get onto it in a bit.

Is there any quick(er) way for me to reach you regarding quick reviews/feedback on things, or should I just be a patient man? ;)

@nanaya
Copy link
Collaborator

nanaya commented Sep 5, 2015

you can reach me in slack. How fast I will respond is a bit of mystery though.

@nanaya nanaya changed the title Issue 45 User menu Sep 5, 2015
- Replaced gamemode icon (png) with the font version.
- Now including country flags automatically (NOTE: I'm using the path "/images/flags/{{ Auth::user()->country_acronym }}.png")
- Reworked the ENTIRE dropdown modal for both login and when logged in, to be based upon the BEM concept.
- Now using localization

As discussed with @nanaya on Slack, this entire has been reworked to be based upon the BEM concept:
This also means that there is now a BEM folder, and a bem-index.less file, in which each BEM-block is being included by it's own .less file.
This can be changed in the future, but for now I suppose this is okay. It's pretty much a playtest for now, doesn't really matter as it's easily moveable.
@claudiodekker
Copy link
Contributor Author

Fixed most issues mentioned, and did some (on slack) discussed stuff..

  • Replaced gamemode icon (png) with the font version.
  • Now including country flags properly and directly from the existing flag icons.
  • Reworked the entire dropdown modal for both login and when logged in, to be based upon the BEM concept.
  • Now using localization

As discussed with @nanaya on Slack, this entire has been reworked to be based upon the BEM concept:

This also means that there is now a BEM folder, and a bem-index.less file, in which each BEM-block is being included by it's own .less file.

This can be changed relatively easy in the future, but for now I suppose this is okay.

@nanaya
Copy link
Collaborator

nanaya commented Sep 9, 2015

sorry for late response,

  • there are merge conflicts
  • files should have end of lines
  • less files will have two spaces indentations from now on (following common style)

text-align: center;
display: block;

&--small {

This comment was marked as off-topic.

This comment was marked as off-topic.


.text-right {
font-weight: 600;
text-align: right;

This comment was marked as off-topic.

This comment was marked as off-topic.

@@ -63,7 +60,8 @@
],
],
'first_members' => 'here since the beginning',
'is_supporter' => 'osu!supporter',
'is_supporter' => 'osu!Supporter',

This comment was marked as off-topic.

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.

4 participants