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

Better layout for bg and theme selectors using flex #1448

Merged
merged 2 commits into from
Jun 29, 2016

Conversation

pgiraud
Copy link
Contributor

@pgiraud pgiraud commented Jun 22, 2016

@rbovard
Copy link
Contributor

rbovard commented Jun 22, 2016

Is it not better to have first images and then descriptions? Because currently the images are more closer to the other column...

Current
image

Proposition
image

@pgiraud
Copy link
Contributor Author

pgiraud commented Jun 22, 2016

Nice suggestion @rbovard. And it will make it easier to customize (no HTML changes) if you want the text below the image like it's done in 1.6.

@pgiraud pgiraud changed the title Better layout for bg and theme selectors using flex [WIP] Better layout for bg and theme selectors using flex Jun 22, 2016
@pgiraud pgiraud force-pushed the selectors_layout branch 2 times, most recently from 90a84ed to 4cbc658 Compare June 22, 2016 13:13
@pgiraud
Copy link
Contributor Author

pgiraud commented Jun 22, 2016

@rbovard suggestion taken into account.
I simplified the selectors partials a bit (using an "active" class name instead of "-check").
An finally I took the liberty to customize the alternative desktop app a bit with:

  • different colors,
  • different layout in the theme selector.

Enjoy and review please.

Démos:
https://pgiraud.github.io/ngeo/selectors_layout/examples/contribs/gmf/apps/desktop
https://pgiraud.github.io/ngeo/selectors_layout/examples/contribs/gmf/apps/desktop_alt

@rbovard
Copy link
Contributor

rbovard commented Jun 22, 2016

I enjoyed! Thanks for this nice work :)

@pgiraud pgiraud changed the title [WIP] Better layout for bg and theme selectors using flex Better layout for bg and theme selectors using flex Jun 22, 2016
}
display: flex;
align-items: center;
min-height: 30px;
Copy link
Member

Choose a reason for hiding this comment

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

Why px here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the thumbnails are images. The size of the images are given in pixels. This value prevents the blank background layer to be displayed with a completely different layout.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By the way, this min-height should go into .gmf-thumbnail.

Copy link
Member

Choose a reason for hiding this comment

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

Because the thumbnails are images. The size of the images are given in pixels. This value prevents the blank background layer to be displayed with a completely different layout.

No relation about using px or rem, and the image is 60px height...

@ger-benjamin
Copy link
Member

Only one question. Then it's ready to merge IMO.

@ger-benjamin
Copy link
Member

Just don't forget to relate this PR to an issue.

@pgiraud
Copy link
Contributor Author

pgiraud commented Jun 23, 2016

See camptocamp/c2cgeoportal#2022

@pgiraud
Copy link
Contributor Author

pgiraud commented Jun 29, 2016

Comments taken into account. Merging. Thanks for the reviews.

@pgiraud pgiraud merged commit e00353c into camptocamp:master Jun 29, 2016
@pgiraud pgiraud deleted the selectors_layout branch June 29, 2016 08:55
@sbrunner sbrunner added this to the Older milestone Aug 23, 2019
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.

None yet

4 participants