From 0e5c88f12132513c881816812733a51c2d69d3a9 Mon Sep 17 00:00:00 2001 From: Jinhao Date: Fri, 24 Jul 2015 00:44:35 +0800 Subject: [PATCH] code review --- include/nana/gui/widgets/checkbox.hpp | 1 - include/nana/gui/widgets/date_chooser.hpp | 1 - include/nana/gui/widgets/menubar.hpp | 1 - include/nana/gui/widgets/progress.hpp | 1 - source/gui/widgets/categorize.cpp | 37 ++++------ source/gui/widgets/checkbox.cpp | 18 +++-- source/gui/widgets/date_chooser.cpp | 67 ++++++++--------- source/gui/widgets/listbox.cpp | 41 ++++------- source/gui/widgets/menubar.cpp | 88 ++++++++++++++++++----- source/gui/widgets/progress.cpp | 17 ++--- source/gui/widgets/treebox.cpp | 6 +- 11 files changed, 152 insertions(+), 126 deletions(-) diff --git a/include/nana/gui/widgets/checkbox.hpp b/include/nana/gui/widgets/checkbox.hpp index 021ab0a1..c0ebee4f 100644 --- a/include/nana/gui/widgets/checkbox.hpp +++ b/include/nana/gui/widgets/checkbox.hpp @@ -37,7 +37,6 @@ namespace drawerbase public: implement * impl() const; private: - void _m_draw(graph_reference); void _m_draw_background(graph_reference); void _m_draw_checkbox(graph_reference, unsigned first_line_height); void _m_draw_title(graph_reference); diff --git a/include/nana/gui/widgets/date_chooser.hpp b/include/nana/gui/widgets/date_chooser.hpp index 9fc0ca8d..9a168bfb 100644 --- a/include/nana/gui/widgets/date_chooser.hpp +++ b/include/nana/gui/widgets/date_chooser.hpp @@ -45,7 +45,6 @@ namespace nana void week_name(unsigned index, const nana::string&); private: where _m_pos_where(graph_reference, const ::nana::point& pos); - void _m_draw(graph_reference); void _m_draw_topbar(graph_reference); void _m_make_drawing_basis(drawing_basis&, graph_reference, const nana::point& refpos); void _m_draw_pos(drawing_basis &, graph_reference, int x, int y, const nana::string&, bool primary, bool sel); diff --git a/include/nana/gui/widgets/menubar.hpp b/include/nana/gui/widgets/menubar.hpp index 44a4913e..8e7270bc 100644 --- a/include/nana/gui/widgets/menubar.hpp +++ b/include/nana/gui/widgets/menubar.hpp @@ -67,7 +67,6 @@ namespace nana bool _m_close_menu(); std::size_t _m_item_by_pos(const ::nana::point&); bool _m_track_mouse(const ::nana::point&); - void _m_draw(); private: widget *widget_; nana::paint::graphics *graph_; diff --git a/include/nana/gui/widgets/progress.hpp b/include/nana/gui/widgets/progress.hpp index 3faaca68..b4f692db 100644 --- a/include/nana/gui/widgets/progress.hpp +++ b/include/nana/gui/widgets/progress.hpp @@ -36,7 +36,6 @@ namespace nana void attached(widget_reference, graph_reference) override; void refresh(graph_reference) override; private: - void _m_draw(); void _m_draw_box(graph_reference); void _m_draw_progress(graph_reference); bool _m_check_changing(unsigned) const; diff --git a/source/gui/widgets/categorize.cpp b/source/gui/widgets/categorize.cpp index e891afea..4351a719 100644 --- a/source/gui/widgets/categorize.cpp +++ b/source/gui/widgets/categorize.cpp @@ -156,10 +156,10 @@ namespace nana { rectangle r{ graph.size() }; - graph.rectangle(r, false, { 0xf0, 0xf0, 0xf0 }); + graph.rectangle(r, false, static_cast(0xf0f0f0)); - color lb(0x9d, 0xab, 0xb9); - color tr(0x48, 0x4e, 0x55); + color lb(static_cast(0x9dabb9)); + color tr(static_cast(0x484e55)); graph.frame_rectangle(r.pare_off(1), lb, tr, tr, lb); } private: @@ -168,7 +168,7 @@ namespace nana const unsigned half = (height - 2) / 2; int left = x + 1; int top = y + 1; - nana::color clr_top(0xea, 0xea, 0xea), clr_bottom(0xdc, 0xdc, 0xdc); + nana::color clr_top(static_cast(0xEAEAEA)), clr_bottom(static_cast(0xDCDCDC)); switch(state) { case mouse_action::over: @@ -192,7 +192,7 @@ namespace nana int bottom = y + height - 1; int right = x + width - 1; - graph.set_color(color(0x6e, 0x8d, 0x9f)); + graph.set_color(static_cast(0x6E8D9F)); graph.line(point{ x, y }, point{right, y}); graph.line(point{ x, y + 1 }, point{ x, bottom }); ++x; @@ -207,8 +207,6 @@ namespace nana ui_element ui_el_; struct style_tag { - //nana::color_t bgcolor; - //nana::color_t fgcolor; color bgcolor; color fgcolor; }style_; @@ -217,8 +215,8 @@ namespace nana class tree_wrapper { public: - typedef widgets::detail::tree_cont container; - typedef container::node_type * node_handle; + using container = widgets::detail::tree_cont; + using node_handle = container::node_type*; tree_wrapper() :splitstr_(STR("\\")), cur_(nullptr) @@ -495,12 +493,11 @@ namespace nana bool erase_locate() { ui_el_.index = npos; - if(ui_el_.what != ui_el_.none) - { - ui_el_.what = ui_el_.none; - return true; - } - return false; + if(ui_el_.what == ui_el_.none) + return false; + + ui_el_.what = ui_el_.none; + return true; } ui_element locate() const @@ -528,13 +525,8 @@ namespace nana if(style_.mode != mode::floatlist) { style_.state = mouse_action::normal; - switch(ui_el_.what) - { - case ui_element::item_name: + if (ui_element::item_name == ui_el_.what) _m_selected(treebase_.tail(ui_el_.index)); - break; - default: break; - } } } @@ -882,9 +874,8 @@ namespace nana void trigger::_m_event_agent_ready() const { - auto & evt = scheme_->evt_holder(); auto evt_agent = event_agent_.get(); - evt.selected = [evt_agent](::nana::any& val){ + scheme_->evt_holder().selected = [evt_agent](::nana::any& val){ evt_agent->selected(val); }; } diff --git a/source/gui/widgets/checkbox.cpp b/source/gui/widgets/checkbox.cpp index d672726f..a5f3720b 100644 --- a/source/gui/widgets/checkbox.cpp +++ b/source/gui/widgets/checkbox.cpp @@ -48,29 +48,35 @@ namespace checkbox void drawer::refresh(graph_reference graph) { - _m_draw(graph); + _m_draw_background(graph); + _m_draw_title(graph); + _m_draw_checkbox(graph, graph.text_extent_size(STR("jN"), 2).height + 2); } void drawer::mouse_down(graph_reference graph, const arg_mouse&) { - _m_draw(graph); + refresh(graph); + API::lazy_refresh(); } void drawer::mouse_up(graph_reference graph, const arg_mouse&) { if(impl_->react) impl_->crook.reverse(); - _m_draw(graph); + refresh(graph); + API::lazy_refresh(); } void drawer::mouse_enter(graph_reference graph, const arg_mouse&) { - _m_draw(graph); + refresh(graph); + API::lazy_refresh(); } void drawer::mouse_leave(graph_reference graph, const arg_mouse&) { - _m_draw(graph); + refresh(graph); + API::lazy_refresh(); } drawer::implement * drawer::impl() const @@ -78,6 +84,7 @@ namespace checkbox return impl_; } + /* void drawer::_m_draw(graph_reference graph) { _m_draw_background(graph); @@ -85,6 +92,7 @@ namespace checkbox _m_draw_checkbox(graph, graph.text_extent_size(STR("jN"), 2).height + 2); API::lazy_refresh(); } + */ void drawer::_m_draw_background(graph_reference graph) { diff --git a/source/gui/widgets/date_chooser.cpp b/source/gui/widgets/date_chooser.cpp index 6679d3d0..7da5474c 100644 --- a/source/gui/widgets/date_chooser.cpp +++ b/source/gui/widgets/date_chooser.cpp @@ -80,37 +80,6 @@ namespace nana return where::none; } - void trigger::_m_draw(graph_reference graph) - { - const unsigned width = graph.width() - 2; - - graph.rectangle(false, {0xb0, 0xb0, 0xb0}); - graph.rectangle({ 1, 1, width, static_cast(topbar_height) }, true, colors::white); - - _m_draw_topbar(graph); - - if(graph.height() > 2 + topbar_height) - { - nana::point refpos(1, static_cast(topbar_height) + 1); - - nana::paint::graphics gbuf({ width, graph.height() - 2 - topbar_height }); - gbuf.rectangle(true, {0xf0, 0xf0, 0xf0}); - - switch(page_) - { - case page::date: - _m_draw_days(refpos, gbuf); - break; - case page::month: - _m_draw_months(refpos, gbuf); - break; - default: break; - } - - graph.bitblt(refpos.x, refpos.y, gbuf); - } - } - void trigger::_m_draw_topbar(graph_reference graph) { ::nana::color arrow_bgcolor; @@ -455,7 +424,33 @@ namespace nana void trigger::refresh(graph_reference graph) { - _m_draw(graph); + const unsigned width = graph.width() - 2; + + graph.rectangle(false, { 0xb0, 0xb0, 0xb0 }); + graph.rectangle({ 1, 1, width, static_cast(topbar_height) }, true, colors::white); + + _m_draw_topbar(graph); + + if (graph.height() > 2 + topbar_height) + { + nana::point refpos(1, static_cast(topbar_height)+1); + + nana::paint::graphics gbuf({ width, graph.height() - 2 - topbar_height }); + gbuf.rectangle(true, { 0xf0, 0xf0, 0xf0 }); + + switch (page_) + { + case page::date: + _m_draw_days(refpos, gbuf); + break; + case page::month: + _m_draw_months(refpos, gbuf); + break; + default: break; + } + + graph.bitblt(refpos.x, refpos.y, gbuf); + } } void trigger::attached(widget_reference widget, graph_reference) @@ -468,7 +463,7 @@ namespace nana where pos = _m_pos_where(graph, arg.pos); if(pos == pos_ && pos_ != where::textarea) return; pos_ = pos; - _m_draw(graph); + refresh(graph); API::lazy_refresh(); } @@ -476,7 +471,7 @@ namespace nana { if(where::none == pos_) return; pos_ = where::none; - _m_draw(graph); + refresh(graph); API::lazy_refresh(); } @@ -595,7 +590,7 @@ namespace nana nana::paint::graphics dirtybuf({ r.width, r.height }); dirtybuf.bitblt(r, graph, refpos); - _m_draw(graph); + refresh(graph); nana::paint::graphics gbuf({ r.width, r.height }); gbuf.bitblt(r, graph, refpos); @@ -603,7 +598,7 @@ namespace nana _m_perf_transform(tfid, graph, dirtybuf, gbuf, refpos); } else - _m_draw(graph); + refresh(graph); API::lazy_refresh(); } diff --git a/source/gui/widgets/listbox.cpp b/source/gui/widgets/listbox.cpp index 38faca94..8487bb59 100644 --- a/source/gui/widgets/listbox.cpp +++ b/source/gui/widgets/listbox.cpp @@ -858,13 +858,6 @@ namespace nana return &list_.back(); } - /// just append a list of new cat. - void create_cat(const std::initializer_list& args) - { - for (auto & arg : args) - list_.emplace_back(arg); - } - /// will use the key to insert new cat before the first cat with compare less than the key, or at the end of the list of cat and return a ref to that new cat. ? category_t* create_cat(std::shared_ptr ptr) { @@ -1080,17 +1073,10 @@ namespace nana std::vector& get_cells(category_t * cat, size_type pos) const { - if (!cat || pos >= cat->items.size()) - throw std::out_of_range("nana::listbox: bad item position"); + if (!cat) + throw std::out_of_range("nana::listbox: category is null"); - return cat->items[pos].cells; - } - - nana::string text(category_t* cat, size_type pos, size_type col) const - { - if (pos < cat->items.size() && (col < cat->items[pos].cells.size())) - return cat->items[pos].cells[col].text; - return{}; + return cat->items.at(pos).cells; } void text(category_t* cat, size_type pos, size_type col, cell&& cl, size_type columns) @@ -3745,7 +3731,7 @@ namespace nana nana::string item_proxy::text(size_type col) const { - return ess_->lister.text(cat_, pos_.item, col); + return ess_->lister.get_cells(cat_, pos_.item).at(col).text; } void item_proxy::icon(const nana::paint::image& img) @@ -3765,17 +3751,18 @@ namespace nana //Behavior of Iterator's value_type bool item_proxy::operator==(const nana::string& s) const { - return (ess_->lister.text(cat_, pos_.item, 0) == s); + return (ess_->lister.get_cells(cat_, pos_.item).at(0).text == s); } bool item_proxy::operator==(const char * s) const { - return (ess_->lister.text(cat_, pos_.item, 0) == nana::string(nana::charset(s))); + return (ess_->lister.get_cells(cat_, pos_.item).at(0).text == nana::string(nana::charset(s))); + } bool item_proxy::operator==(const wchar_t * s) const { - return (ess_->lister.text(cat_, pos_.item, 0) == nana::string(nana::charset(s))); + return (ess_->lister.get_cells(cat_, pos_.item).at(0).text == nana::string(nana::charset(s))); } item_proxy & item_proxy::operator=(const item_proxy& rhs) @@ -3792,10 +3779,9 @@ namespace nana // Behavior of Iterator item_proxy & item_proxy::operator++() { - if (++pos_.item < cat_->items.size()) - return *this; + if (++pos_.item >= cat_->items.size()) + cat_ = nullptr; - cat_ = nullptr; return *this; } @@ -3916,8 +3902,7 @@ namespace nana for (item_proxy &it : *this ) it.select(sel); - ess_->lister.last_selected_abs = - ess_->lister.last_selected_dpl = index_pair {this->pos_, npos}; + ess_->lister.last_selected_abs = ess_->lister.last_selected_dpl = index_pair {this->pos_, npos}; return *this; } @@ -4232,7 +4217,9 @@ namespace nana { internal_scope_guard lock; auto & ess = _m_ess(); - ess.lister.create_cat(args); + + for (auto & arg : args) + ess.lister.create_cat(nana::string{ arg }); ess.update(); } diff --git a/source/gui/widgets/menubar.cpp b/source/gui/widgets/menubar.cpp index 2c1de33d..203de8ec 100644 --- a/source/gui/widgets/menubar.cpp +++ b/source/gui/widgets/menubar.cpp @@ -148,7 +148,7 @@ namespace nana auto pos = items_->cont().size(); items_->append(text, shkey); - _m_draw(); + refresh(*graph_); API::update_window(*widget_); return at(pos); @@ -173,13 +173,65 @@ namespace nana widget_ = &widget; } - void trigger::refresh(graph_reference) + void trigger::refresh(graph_reference graph) { - _m_draw(); - API::lazy_refresh(); + auto bgcolor = API::bgcolor(*widget_); + graph_->rectangle(true, bgcolor); + + item_renderer ird(*widget_, graph); + + nana::point item_pos(2, 2); + nana::size item_s(0, 23); + + unsigned long index = 0; + for (auto i : items_->cont()) + { + //Transform the text if it contains the hotkey character + ::nana::char_t hotkey; + ::nana::string::size_type hotkey_pos; + auto text = API::transform_shortkey_text(i->text, hotkey, &hotkey_pos); + + nana::size text_s = graph.text_extent_size(text); + + item_s.width = text_s.width + 16; + + i->pos = item_pos; + i->size = item_s; + + using state = item_renderer::state; + state item_state = (index != state_.active ? state::normal : (state_.menu_active ? state::selected : state::highlighted)); + ird.background(item_pos, item_s, item_state); + + if (state::selected == item_state) + { + int x = item_pos.x + item_s.width; + int y1 = item_pos.y + 2, y2 = item_pos.y + item_s.height - 1; + graph.line({ x, y1 }, { x, y2 }, bgcolor.blend(colors::gray_border, 0.4)); + graph.line({ x + 1, y1 }, { x + 1, y2 }, bgcolor.blend(colors::button_face_shadow_end, 0.5)); + } + + //Draw text, the text is transformed from orignal for hotkey character + int text_top_off = (item_s.height - text_s.height) / 2; + ird.caption({ item_pos.x + 8, item_pos.y + text_top_off }, text); + + if (hotkey) + { + unsigned off_w = (hotkey_pos ? graph.text_extent_size(text, static_cast(hotkey_pos)).width : 0); + nana::size hotkey_size = graph.text_extent_size(text.c_str() + hotkey_pos, 1); + + unsigned ascent, descent, inleading; + graph.text_metrics(ascent, descent, inleading); + int x = item_pos.x + 8 + off_w; + int y = item_pos.y + text_top_off + ascent + 1; + graph.line({ x, y }, { x + static_cast(hotkey_size.width) - 1, y }, ::nana::colors::black); + } + + item_pos.x += i->size.width; + ++index; + } } - void trigger::mouse_move(graph_reference, const arg_mouse& arg) + void trigger::mouse_move(graph_reference graph, const arg_mouse& arg) { if (arg.pos != state_.mouse_pos) state_.nullify_mouse = false; @@ -200,7 +252,7 @@ namespace nana if(popup) { _m_popup_menu(); - _m_draw(); + refresh(graph); API::lazy_refresh(); } @@ -228,7 +280,7 @@ namespace nana else _m_total_close(); - _m_draw(); + refresh(graph); API::lazy_refresh(); } @@ -245,12 +297,12 @@ namespace nana { state_.behavior = state_.behavior_none; _m_total_close(); - _m_draw(); + refresh(graph); API::lazy_refresh(); } } - void trigger::focus(graph_reference, const arg_focus& arg) + void trigger::focus(graph_reference graph, const arg_focus& arg) { if((arg.getting == false) && (state_.active != npos)) { @@ -259,12 +311,12 @@ namespace nana state_.menu_active = false; _m_close_menu(); state_.active = npos; - _m_draw(); + refresh(graph); API::lazy_refresh(); } } - void trigger::key_press(graph_reference, const arg_keyboard& arg) + void trigger::key_press(graph_reference graph, const arg_keyboard& arg) { state_.nullify_mouse = true; if(state_.menu) @@ -354,11 +406,11 @@ namespace nana } } - _m_draw(); + refresh(graph); API::lazy_refresh(); } - void trigger::key_release(graph_reference, const arg_keyboard& arg) + void trigger::key_release(graph_reference graph, const arg_keyboard& arg) { if(arg.key == 18) { @@ -376,7 +428,7 @@ namespace nana } state_.menu_active = false; - _m_draw(); + refresh(graph); API::lazy_refresh(); } } @@ -396,7 +448,7 @@ namespace nana if(_m_popup_menu()) state_.menu->goto_next(true); - _m_draw(); + refresh(graph); API::lazy_refresh(); state_.behavior = state_.behavior_menu; } @@ -424,7 +476,7 @@ namespace nana if(index != state_.active) { state_.active = index; - _m_draw(); + refresh(*graph_); API::lazy_refresh(); if(_m_popup_menu()) @@ -456,7 +508,7 @@ namespace nana { _m_total_close(); - _m_draw(); + refresh(*graph_); API::update_window(widget_->handle()); } }); @@ -526,6 +578,7 @@ namespace nana return false; } + /* void trigger::_m_draw() { auto bgcolor = API::bgcolor(*widget_); @@ -583,6 +636,7 @@ namespace nana ++index; } } + */ //struct state_type trigger::state_type::state_type() diff --git a/source/gui/widgets/progress.cpp b/source/gui/widgets/progress.cpp index 8b17a124..9f0bcf57 100644 --- a/source/gui/widgets/progress.cpp +++ b/source/gui/widgets/progress.cpp @@ -42,7 +42,7 @@ namespace nana if(_m_check_changing(value_)) { - _m_draw(); + refresh(*graph_); API::update_window(widget_->handle()); } return v; @@ -99,18 +99,13 @@ namespace nana return s; } - void trigger::refresh(graph_reference) + void trigger::refresh(graph_reference graph) { - _m_draw(); - } + if (false == unknown_) + draw_width_ = static_cast((graph.width() - border * 2) * (double(value_) / max_)); - void trigger::_m_draw() - { - if(false == unknown_) - draw_width_ = static_cast((graph_->width() - border * 2) * (double(value_) / max_)); - - _m_draw_box(*graph_); - _m_draw_progress(*graph_); + _m_draw_box(graph); + _m_draw_progress(graph); } void trigger::_m_draw_box(graph_reference graph) diff --git a/source/gui/widgets/treebox.cpp b/source/gui/widgets/treebox.cpp index 98578aad..178120f6 100644 --- a/source/gui/widgets/treebox.cpp +++ b/source/gui/widgets/treebox.cpp @@ -27,7 +27,7 @@ namespace nana //Here defines some function objects namespace treebox { - typedef trigger::node_type node_type; + using node_type = trigger::node_type; bool no_sensitive_compare(const nana::string& text, const nana::char_t *pattern, std::size_t len) { @@ -56,7 +56,7 @@ namespace nana node = node->child; while(node) { - if(no_sensitive_compare(node->value.second.text, pattern, len)) return node; + if(no_sensitive_compare(node->value.second.text, pattern, len)) return node; if(node == end) break; @@ -78,7 +78,7 @@ namespace nana : public drawer_trigger, public compset_interface { public: - typedef drawer_trigger::graph_reference graph_reference; + using graph_reference = drawer_trigger::graph_reference; void assign(const item_attribute_t & item_attr, const pat::cloneable* renderer, const pat::cloneable * compset_placer) {