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

Val dev merged reordered #219

Open
wants to merge 65 commits into
base: develop
Choose a base branch
from

Conversation

4xdk
Copy link
Collaborator

@4xdk 4xdk commented Jun 30, 2018

Hopefully I didn't remove anything that was needed.
This is related to issues #122 and #123

-moz-user-select: none; /* Firefox */
-ms-user-select: none; /* Internet Explorer/Edge */
user-select: none; /* Non-prefixed version, currently supported by Chrome and Opera */
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@4xdk Do you see this used anywhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah nice spot. I've used it on the menu but it went missing during refactoring I'll add it back to nav-pursuances!

action='about'
icon={<Info size={28} />}
/>
<PursuanceMenuItem
Copy link
Contributor

Choose a reason for hiding this comment

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

@4xdk The latest version of this (in develop) is different; we no longer need to pass in pursuanceId and location explicitly, as PursuanceMenuItem is now reduxified.

||
location.pathname.indexOf(`/${action}`) !== -1
))
)
Copy link
Contributor

Choose a reason for hiding this comment

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

There is also more to this function now in develop; it treats the discuss action differently, which is what we want.

font-weight: normal;
}

.custom-dropdown {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This dropdown doesn't seem to match the style of other dropdowns, but still looks good IMO.

screenshot_20180706_200128

As an aside, I couldn't find a way to navigate to pursuances/all normally.

@4xdk
Copy link
Collaborator Author

4xdk commented Jul 7, 2018

I've updated to match site dropdown style, also changed to say 'sort' instead.
screenshot_20180707_213420

}

.custom-dropdown::before {
background-color: rgba(0,0,0,.15);
#dashboard .sort .dropdown.open > .dropdown-menu {
Copy link
Contributor

Choose a reason for hiding this comment

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

@4xdk Do you like using the > for efficiency? It makes things more brittle, but I don't know about the speed differences.

Copy link
Collaborator Author

@4xdk 4xdk Jul 26, 2018

Choose a reason for hiding this comment

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

TBH I just prefer to be reasonably specific to avoid styling something I didn't intend to. Especially working with unfamiliar code or overly generic classes as is the case here.

I wouldn't worry about CSS performance at all until we get to some serious animations.

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.

3 participants