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.

Sign up for GitHub

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

Jump to bottom

ui update #879

Merged
merged 3 commits into from
Aug 8, 2016
Merged

ui update #879

merged 3 commits into from
Aug 8, 2016

Conversation

ascott
Copy link
Contributor

@ascott ascott commented Aug 4, 2016

  • simplify caravel branding, wordmark only
  • get rid of panel borders and shadows
  • updates to panel headings
  • update explore header for better ux/layout
  • lots of small styling tweaks
  • this will continue to be iterated on

plz review @mistercrunch @williaster

screenshot 2016-08-04 12 31 24

caravel - dashboard-8-4

caravel-slice-8-4

@coveralls
Copy link

coveralls commented Aug 4, 2016 •
edited
Loading

Coverage Status

Coverage remained the same at 81.494% when pulling 35f289e3f5bb35642fe61468fa957be1e63c2963 on ascott:alanna-font-exploration into 82a8e63 on airbnb:master.

@coveralls
Copy link

coveralls commented Aug 4, 2016 •
edited
Loading

Coverage Status

Coverage remained the same at 81.494% when pulling 35f289e3f5bb35642fe61468fa957be1e63c2963 on ascott:alanna-font-exploration into 82a8e63 on airbnb:master.

@mistercrunch
Copy link
Member

img

@mistercrunch
Copy link
Member

img

@mistercrunch
Copy link
Member

from the screenshots the only thing I'd change is the active page 1 (white on black) that pops a bit much. Maybe white on gray?

@williaster
Copy link
Contributor

yeah this looks amazing overall! agree with the intense button on the landing page.

@williaster
Copy link
Contributor

minimalism++

@alanmcruickshank
Copy link
Contributor

I like this a lot. Awesome work!

On Fri, 5 Aug 2016 17:33 Chris Williams, notifications@github.com wrote:

minimalism++

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#879 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AEdFuDbxlxN_Zf2Jk0xPZ4VBZrJKXJ_oks5qc2XVgaJpZM4JdDPq
.

Alan Cruickshank
E: alanmcruickshank@gmail.com
T: +44 770 6851825

@ascott
Copy link
Contributor Author

ascott commented Aug 5, 2016

good point on the pagination styles. will fix.

@coveralls
Copy link

coveralls commented Aug 5, 2016 •
edited
Loading

Coverage Status

Coverage remained the same at 81.488% when pulling 23bc8b4 on ascott:alanna-font-exploration into 7c810db on airbnb:master.

@coveralls
Copy link

coveralls commented Aug 5, 2016 •
edited
Loading

Coverage Status

Coverage remained the same at 81.488% when pulling e9dba07 on ascott:alanna-font-exploration into 7c810db on airbnb:master.

@ascott
Copy link
Contributor Author

ascott commented Aug 8, 2016

@mistercrunch do you know why the code-quality/landscape check is still pending?

@ascott
Copy link
Contributor Author

ascott commented Aug 8, 2016

ah, just missed your comment in slack about this @mistercrunch. merging since landscape is being flaky.

@ascott ascott merged commit b0a1f07 into apache:master Aug 8, 2016
@ascott ascott deleted the alanna-font-exploration branch August 8, 2016 17:55
ascott pushed a commit that referenced this pull request Aug 8, 2016
ui update (#879)
cedbce1 
* caravel ui update

* make headings bold on /explore

* bump back pagination color
bkyryliuk pushed a commit that referenced this pull request Aug 10, 2016
ui update (#879)
cab54c4 
* caravel ui update

* make headings bold on /explore

* bump back pagination color
the-dcruz
the-dcruz reviewed Aug 10, 2016
View reviewed changes
caravel/assets/package.json
@@ -38,7 +38,6 @@
"autobind-decorator": "^1.3.3",
"bootstrap": "^3.3.6",
"bootstrap-datepicker": "^1.6.0",
"bootstrap-toggle": "^2.2.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this removed intentionally? It is still required in explore.jsx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i replaced the toggles in expore.jsx with simple checkboxes, but maybe i missed an instance?

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha. Sounds like the requires for toggle can be removed then?

Copy link
Contributor Author

@ascott ascott Aug 11, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

good catch, i'll get a pr in to fix this tonight.
https://github.com/airbnb/caravel/blob/master/caravel/assets/javascripts/explore/explore.jsx#L21

@ascott ascott mentioned this pull request Aug 11, 2016
Closed
@szmate1618 szmate1618 mentioned this pull request Dec 16, 2016
Merged
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.11.0 labels Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.11.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ascott @coveralls @mistercrunch @williaster @alanmcruickshank @the-dcruz